[Toybox] [TOY] lspci

Georgi Chorbadzhiyski gf at unixsol.org
Mon Jul 22 12:18:38 PDT 2013


On 7/22/13 10:16 PM, 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).

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;

-- 
Georgi Chorbadzhiyski
http://georgi.unixsol.org/
http://github.com/gfto/



More information about the Toybox mailing list