<br><br>
<div class="gmail_quote">On Tue, Jul 23, 2013 at 6:20 AM, Felix Janda <span dir="ltr"><<a href="mailto:felix.janda@posteo.de" target="_blank">felix.janda@posteo.de</a>></span> wrote:<br>
<blockquote style="BORDER-LEFT:#ccc 1px solid;MARGIN:0px 0px 0px 0.8ex;PADDING-LEFT:1ex" class="gmail_quote">
<div class="HOEnZb">
<div class="h5">Georgi Chorbadzhiyski wrote:<br>> On 7/22/13 10:05 PM, Felix Janda wrote:<br>> > Isaac wrote:<br>> >><br>> >> I've written an lspci implementation.<br>> ><br>> > Cool.<br>
> ><br>> >> Currently it supports -emkn; -e is an extension ("class" is a 24-bit number,<br>> >> but lspci only shows 16 bits; one person on the Puppy forums mentioned<br>> >> that they need those last 8 bits).<br>
> >> -n is a no-op for compatability with standard lspci.<br>> ><br>> > Are there any more options you have in mind that should be implemented?<br>> > Just out of curiosity: Could you post a link to the forum post?<br>
> ><br>> >> 75 lines total.<br>> ><br>> > Down to 70 after application of the below patch with small fixes.<br>> ><br>> > It felt more sensible to me to read 2 more bytes and ignore them than to<br>
> > seek for 2 bytes in a file. So preadat_name now has one argument less.<br>> ><br>> > Use calloc instead of malloc + memset. With this also the off-by-one<br>> > error from (origninally) line 32 is fixed. It would be nice to use<br>
> > toybuf for this, wouldn't it? That would also make valgrind happier.<br>> ><br>> > Print "" instead of "." when .../driver is missing.<br>> ><br>> > The only difference between the usual output and the machine readable<br>
> > output is the format string. Use this to avoid some repetition.<br>> ><br>> > Felix<br>> ><br>> > --- a/lspci.c       2013-07-22 18:05:37.514838360 +0200<br>> > +++ b/lspci.c       2013-07-22 19:12:03.000000000 +0200<br>
> > @@ -17,19 +17,14 @@<br>> >  */<br>> >  #define FOR_lspci<br>> >  #include "toys.h"<br>> > -char * preadat_name(int dirfd, char *fname, size_t nbyte, off_t offset)<br>> > +char * preadat_name(int dirfd, char *fname, size_t nbyte)<br>
> >  {<br>> >    int fd;<br>> > -  char *buf = malloc(nbyte+1);<br>> > -  memset(buf, 0, sizeof(buf));<br>> > -  fd = openat(dirfd, fname, O_RDONLY);<br>> > -  if (fd < 0) {<br>> > -    return NULL;<br>
> > -  }<br>> > -  lseek(fd, offset, SEEK_SET);<br>> > +  char *buf = calloc(1, nbyte+1);<br>> > +<br>> > +  if (0 > (fd = openat(dirfd, fname, O_RDONLY))) return NULL;<br>><br>> It would be better to move calloc after openat (which may fail).<br>
<br></div></div>That's true. Actually I'd prefer having no dynamic allocation at all.<br>Have something like<br><br>struct {<br>  char class[8], vendor[6], device[6], module[256];<br>};<br><br>at toybuf and use that as the buffer. Maybe also remove preadat<br>
all together and use a loop in do_lspci?<br>
<div class="im"><br>Georgi Chorbadzhiyski wrote:<br>> And another thing, IMHO<br>><br>> ((fd = openat(dirfd, fname, O_RDONLY)) < 0) return NULL;<br>><br>> is better than<br>><br></div>
<div class="im">> if (0 > (fd = openat(dirfd, fname, O_RDONLY))) return NULL;<br><br></div>I agree.<br><span class="HOEnZb"><font color="#888888"><br>Felix<br></font></span>
<div class="HOEnZb">
<div class="h5">______</div></div></blockquote>
<div> </div>
<div> </div>
<div>  hi,</div>
<div>         I have some questions actually :-</div>
<div> </div>
<div> > > -  fd = openat(dirfd, fname, O_RDONLY);<br>> > -  if (fd < 0) {<br>> > -    return NULL;<br>> > -  }<br>> > -  lseek(fd, offset, SEEK_SET);<br>> > +  char *buf = calloc(1, nbyte+1);<br>
> > +<br>            why calloc and why not __xzalloc__ ?  Are not we following toybox coding style.</div>
<div> </div>
<div>  And what if _calloc_ returns NULL. we will be  using a NULL buffer, it may lead to a potentional crash.</div>
<div> </div>
<div>--</div>
<div>Sandeep</div>
<div> </div>
<div> </div>
<div> </div>
<blockquote style="BORDER-LEFT:#ccc 1px solid;MARGIN:0px 0px 0px 0.8ex;PADDING-LEFT:1ex" class="gmail_quote">
<div class="HOEnZb">
<div class="h5">_________________________________________<br>Toybox mailing list<br><a href="mailto:Toybox@lists.landley.net">Toybox@lists.landley.net</a><br><a href="http://lists.landley.net/listinfo.cgi/toybox-landley.net" target="_blank">http://lists.landley.net/listinfo.cgi/toybox-landley.net</a><br>
</div></div></blockquote></div><br>