libxo for devinfo #1450
libxo for devinfo #1450
Conversation
|
Oh! This is one on my list! I'll review this in detail, but it may be towards the end of the week... |
|
Thanks @bsdimp , I actually got it from the list of ideas on your website. |
| printf(" "); | ||
| printf("%s:\n", rman->dm_desc); | ||
|
|
||
| safe_desc = (char*) malloc(sizeof(rman->dm_desc)); |
|
|
||
| /* print resources */ | ||
| ia->indent = indent + 4; | ||
| devinfo_foreach_rman_resource(rman, | ||
| print_device_matching_resource, ia); | ||
| print_device_matching_resource, ia); |
There was a problem hiding this comment.
the indentation was right before. We do 4 space continuation lines.
| @@ -168,8 +270,13 @@ print_device(struct devinfo_dev *dev, void *arg) | |||
| } | |||
| } | |||
|
|
|||
| return(devinfo_foreach_device_child(dev, print_device, | |||
| int ret = (devinfo_foreach_device_child(dev, print_device, | |||
There was a problem hiding this comment.
style(9) (man style) says that ret should be declared at the start of the block.
| print_resource(res); | ||
| struct devinfo_rman *rman; | ||
| int hexmode; | ||
| char s[32]; |
There was a problem hiding this comment.
why 32? Is there a #define that can be used here?
|
|
||
| if (hexmode) { | ||
| if (res->dr_size > 1) | ||
| snprintf(s, 32, "0x%lx-0x%lx", |
There was a problem hiding this comment.
sizeof(s) is better. but this isn't 32, but really 5 ("0x-0x") + 1 NUL + 2 * (sizeof(long) * 2) so this can be at most 38, so 32 is a little small... and I think this is the longest one.
| printf("%s:\n", rman->dm_desc); | ||
| char* safe_desc; | ||
|
|
||
| safe_desc = (char*) malloc(sizeof(rman->dm_desc)); |
There was a problem hiding this comment.
same question as above. sizeof vs strlen?
| static void | ||
| print_device_path_entry(struct devinfo_dev *dev, const char* devname) { | ||
| xo_open_container(devname); | ||
| if (open_tag_index >= MAX_OPEN_TAGS) { |
|
So I think this is generally OK. There's a number of mostly style issues, and a few more substantive things that should be easy to fix. Though the commit breakup needs a lot of work. There's a fair number of cleanup changes can be separated out before the XO-ification can be done... |
This was more involved and invasive than I imagened. But I think I've done what I can now, at least without guidance/review. "Works for me".