[Toybox] [TOY] lspci

[Isaac]

On Mon, Jul 22, 2013 at 09:05:26PM +0200, Felix Janda wrote:
> Isaac wrote:
> >
> > I've written an lspci implementation.
> 
> Cool.
> 
> > Currently it supports -emkn; -e is an extension ("class" is a 24-bit number,
> > but lspci only shows 16 bits; one person on the Puppy forums mentioned
> > that they need those last 8 bits).
> > -n is a no-op for compatability with standard lspci.
> 
> Are there any more options you have in mind that should be implemented?

-s DEVICE is highly desireable, but I didn't get it working right.
(-s shows only the device that has a matching end to its name:
-s 03:00.0 shows 0000:03:00.0)

> Just out of curiosity: Could you post a link to the forum post?
> 
http://www.murga-linux.com/puppy/viewtopic.php?t=63971&start=30
"It seems that bb-lspci is not able to replace elspci because it shows 
[ like lspci-FULL ] only the 4 middle chars of /sys/bus/pci/devices/*/class,
 while elspci shows the last 6 chars."

It's mainly that I noticed that, looked in /sys/, figured that an lspci 
that could do that _right_ would be trivial, and wrote it.
> > 75 lines total.
> 
> Down to 70 after application of the below patch with small fixes.
Thanks.

> It felt more sensible to me to read 2 more bytes and ignore them than to
> seek for 2 bytes in a file. So preadat_name now has one argument less.

In which case it should be readat_name.  The "p" is because pread()
accepts an offset.
(I named it for the similarities/diferences from the standard functions:
it's similar to pread(), but it takes a parent directory like openat() and
uses a filename.)

The down side is the longer printf...but that's not worth mentioning, 
I think.

> 
> Use calloc instead of malloc + memset. With this also the off-by-one
> error from (origninally) line 32 is fixed. It would be nice to use
> toybuf for this, wouldn't it? That would also make valgrind happier.
> 
> Print "" instead of "." when .../driver is missing.
> 
> The only difference between the usual output and the machine readable
> output is the format string. Use this to avoid some repetition.
> 
> Felix

Thanks,
Isaac Dunham

> --- a/lspci.c	2013-07-22 18:05:37.514838360 +0200
> +++ b/lspci.c	2013-07-22 19:12:03.000000000 +0200
> @@ -17,19 +17,14 @@
>  */
>  #define FOR_lspci
>  #include "toys.h"
> -char * preadat_name(int dirfd, char *fname, size_t nbyte, off_t offset)
> +char * preadat_name(int dirfd, char *fname, size_t nbyte)
>  {
>    int fd;
> -  char *buf = malloc(nbyte+1);
> -  memset(buf, 0, sizeof(buf));
> -  fd = openat(dirfd, fname, O_RDONLY);
> -  if (fd < 0) {
> -    return NULL;
> -  }
> -  lseek(fd, offset, SEEK_SET);
> +  char *buf = calloc(1, nbyte+1);
> +
> +  if (0 > (fd = openat(dirfd, fname, O_RDONLY))) return NULL;
>    read(fd, buf, nbyte);
>    close(fd);
> -  buf[nbyte +1] = '\0';
>    return buf;
>  }
>  
> @@ -37,31 +32,32 @@
>  {
>    int alen = 8;
>    char *dname = dirtree_path(new, &alen);
> +
>    if (!strcmp("/sys/bus/pci/devices", dname))
>      return DIRTREE_RECURSE;
>    errno = 0;
>    int dirfd = open(dname, O_RDONLY);
>    if (dirfd > 0) {
>      char *class = preadat_name(dirfd, "class",
> -                (toys.optflags & FLAG_e) ? 6 :4, 2);
> -    char *vendor = preadat_name(dirfd, "vendor", 4, 2);
> -    char *device = preadat_name(dirfd, "device", 4, 2);
> +                (toys.optflags & FLAG_e) ? 8 : 6);
> +    char *vendor = preadat_name(dirfd, "vendor", 6);
> +    char *device = preadat_name(dirfd, "device", 6);
> +
>      close(dirfd);
>      if (!errno) {
>        char *driver = "";
> +      char *fmt = toys.optflags & FLAG_m ?  "%s, \"%s\" \"%s\" \"%s\" \"%s\"\n"
> +                                                    : "%s Class %s: %s:%s %s\n";
> +
>        if (toys.optflags & FLAG_k) {
>          char module[256] = "";
> +
>          strcat(dname, "/driver");
> -        readlink(dname, module, sizeof(module));
> -        driver = basename(module);
> +        if (-1 != readlink(dname, module, sizeof(module)))
> +          driver = basename(module);
>        }
> -      if (toys.optflags & FLAG_m) {
> -        printf("%s, \"%s\" \"%s\" \"%s\" \"%s\"\n",new->name + 5, class, 
> -               vendor, device, driver);
> -      } else {
> -        printf("%s Class %s: %s:%s %s\n", new->name + 5, class, vendor, device, 
> +      printf(fmt, new->name + 5, class + 2, vendor + 2, device + 2,
>                 driver);
> -      }
>      }
>    }
>    return 0;
> @@ -69,6 +65,5 @@
>  
>  void lspci_main(void)
>  {
> -  sprintf(toybuf, "/sys/bus/pci/devices");
> -  dirtree_read(toybuf, do_lspci);
> +  dirtree_read("/sys/bus/pci/devices", do_lspci);
>  }
>