Comment 17 for bug 159189

Revision history for this message
Colin Watson (cjwatson) wrote :

Please try to make sure to follow the coding style of the surrounding code. usbutils seems to be largely in the Linux kernel's style, so I had to clean up quite a few things in order to be free enough of distractions to read the actual code changes:

  * various whitespace changes, e.g. consistent use of tabs for indentation
  * write comments as C89-style /* ... */ rather than C++/C99-style //...
  * consistent brace style

I reverted your le16_to_cpu changes, which seemed quite unnecessary.

Your formatting is, to say the least, extremely misleading. updateusblist has code that looks like this:

        if (!(bus = malloc(sizeof(struct usbbusnode))))
                lprintf(0, "Out of memory\n");
                bus->busnum = busnum;
...

This looks just like you forgot to put a brace after the if, but actually it seems to be intentional! Please be disciplined about indentation levels to avoid confusion; good editors can often help with this.

I factored out a bit of duplicate code near the end of findtree.

Don't redeclare file_select locally in treedump.

You misspelled "struct dirent" as "struct direct". This produced a compiler warning:

  ../lsusb.c:2777: warning: passing argument 3 of ‘scandir’ from incompatible pointer type

The recursion case for level in findtree doesn't work right; you need to increment it in the recursive call to findtree, not just before exiting the function (which achieves nothing since it's passed by value).

The condition for path construction in findtree needs to be level >= 1, not level >= 2. (I noticed this by comparing lsusb output before and after this change.)

Other than that, this seems to work well, thanks, and I've uploaded this. I've attached the final debdiff here, and I suggest you read over it for future reference.