On Wednesday 11 November 2009 15:50:21 Souf Oued wrote:
> I tried to write a version of lspci, it only uses sysfs and supports 2
> options: m and k, it dumps data only in numeric form (not uses pci.ids
>  file)

looks like a pretty good start ... some comments:
 - you mix tabs and spaces when indenting
 - printf_vid_did is kind of big for a define ... better to make it a static 
func and let gcc choose to inline it, and using variables that arent 
explicitly based in via the macro args makes things fragile to changes
 - the lack of pointer checking around getenv is scary, especially when 
offsets are blindly added to them
 - the sprintf into did looks weird ... why not use memcpy instead
 - the whole put/get env handling seems wasteful, both in terms of memory, cpu 
usage, and polluting the env in case the applet gets inlined ... but i guess 
abusing the environment as a hash table makes things easier for the rest of 
the code
 - the env code is incorrect at any rate ... the memory given to putenv 
becomes part of the environment, so you cant go freeing/modifying it after you 
use putenv
 - options should really be defined enums instead of sprinkling hardcoded 
numbers through out the code
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to