nvme-print: fix traddr transformation for ave discovery log#3264
nvme-print: fix traddr transformation for ave discovery log#3264igaw merged 1 commit intolinux-nvme:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes transport-address (traddr) rendering for AVE discovery log output by passing the actual binary address bytes to inet_ntop() rather than an address-family string.
Changes:
- Correct
inet_ntop()source argument in stdout AVE discovery log printing. - Correct
inet_ntop()source argument in JSON AVE discovery log printing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| nvme-print-stdout.c | Fixes AVE avetraddr formatting by feeding inet_ntop() the binary traddr field. |
| nvme-print-json.c | Fixes AVE avetraddr JSON formatting by feeding inet_ntop() the binary traddr field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for your fix. Just tested the changes with the dummy data as works correctly and the changes look good. Sorry for the error and previously I could not test the original changes correctly. |
|
Thanks! The copilot review has brought up some good points. I'll add them next week. running out of steam for today. |
inet_ntop wants the network address structure as input not converted string. Fixes: 1571543 ("nvme: add ave-discovery-log command") Signed-off-by: Daniel Wagner <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| socklen_t size; | ||
| int af; |
There was a problem hiding this comment.
socklen_t size is missing a trailing semicolon, which will break compilation. Add ; after the declaration.
inet_ntop wants the network address structure as input not converted string.
Fixes: 1571543 ("nvme: add ave-discovery-log command")
@ikegami-t did you tests the original implementation? If so, can you try this one too?