[Toybox] lsusb: add -t option for tree format display

[Rob Landley]

On 7/20/25 21:25, Rob Landley wrote:
> I can change it here, let me take a closer look this evening. (I'm in 
> tokyo at the moment juggling other commitments...)

"git am" warned of a bunch of trailing whitespace errors, so I did a
sed -i 's/ *$//' toys/*/lsusb.c

Why did you change id1 and id2 to unsigned in get_names()? (The protocol 
fields are 16 bit, int is 32 bit, how can it be negative? Do you have a 
test case this changed the behavior for?)

Since unsigned defaults to int (in C99, probably C89 too) you can just 
say "unsigned" without having to say "unsigned int". (Code style: 
shortest representation for consistency, you only have to specify 
non-default attributes.)

> toys/other/lsusb.c: In function 'create_device':
> toys/other/lsusb.c:207:44: warning: '%s' directive output may be truncated writing 6 bytes into a region of size between 0 and 255 [-Wformat-truncation=]
>   207 |   snprintf(fullpath, sizeof(fullpath), "%s/%s", path, file);
>       |                                            ^~
> In function 'read_sysfs_uint',
>     inlined from 'create_device' at toys/other/lsusb.c:284:17:
> toys/other/lsusb.c:207:3: note: 'snprintf' output between 8 and 263 bytes into a destination of size 256
>   207 |   snprintf(fullpath, sizeof(fullpath), "%s/%s", path, file);
>       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I generally try to have it compile without warnings, even silly ones.

Hmmm, "diff -u <(lsusb -t) <(./lsusb -t)" doesn't share a single common 
line of output. I ran it through "sort" and did "diff -bu" and it's 
still not matching anything. So what's different...

/usr/bin/lsusb -t says:
-/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M

toybox lsusb -t says:
+/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=hub/0p, 480M

I dunno what the "driver" field should be, but it's not the same info.

> // Tree display functions
> static struct usb_bus *usb_buses = NULL;

Globals go in the GLOBALS() block, it's a memory efficiency thing. (See 
https://landley.net/toybox/code.html#toy_union .)

You don't need to initialize global or static variables, ELF insures 
they default to null/zero when sticking them in the .bss section.

Speaking of your struct types with the fixed size buffers, notice the 
difference between:

struct usb_bus {
   struct usb_bus *next;
   struct usb_device *first_child;
   unsigned busnum;
   char name[64];
};

struct dev_ids {
   struct dev_ids *next, *child;
   int id;
   char name[];
};

Two things:

1) Those are functionally the same struct, tree of int with string, you 
could presumably have reused dev_ids. (Ok, avoids a typecast for the 
child...)

2) Mine had a dynamically sized string field at the end so malloc() can 
measure the string it's about to copy and allocate enough space. Yours 
is doing a fixed length buffer which you trust /proc never to provide 
too much data to. (I don't trust /proc not to have random crap --bind 
mounted over it in container du jour, things like scan_uevent() trusting 
new->name to fit in 256 bytes with the null terminator is because that 
data came from readdir() which came getdents() which came from the VFS 
which limits each path component size to a max length of 255 bytes and 
says it can't contain / or NUL.)

Gotta come back to this later...

> Thanks,
> 
> Rob

Rob