[Toybox] [TOY] lspci

[Felix Janda]

Georgi Chorbadzhiyski wrote:
> On 7/22/13 10:05 PM, 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?
> > Just out of curiosity: Could you post a link to the forum post?
> > 
> >> 75 lines total.
> > 
> > Down to 70 after application of the below patch with small fixes.
> > 
> > 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.
> > 
> > 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
> > 
> > --- 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;
> 
> It would be better to move calloc after openat (which may fail).

That's true. Actually I'd prefer having no dynamic allocation at all.
Have something like

struct {
  char class[8], vendor[6], device[6], module[256];
};

at toybuf and use that as the buffer. Maybe also remove preadat
all together and use a loop in do_lspci?

Georgi Chorbadzhiyski wrote:
> And another thing, IMHO
> 
> ((fd = openat(dirfd, fname, O_RDONLY)) < 0) return NULL;
> 
> is better than
> 
> if (0 > (fd = openat(dirfd, fname, O_RDONLY))) return NULL;

I agree.

Felix