Am 16.06.2010 09:35, schrieb Carl-Daniel Hailfinger: > Could you make perror() a wrapper for strerror()? flashrom will move to > strerror() soon to behave more like a library. We don't have sterror, so I simply let it print the error code (which isn't set anywhere). This is stubbed until there's actually some real user of errno.
>> Signed-off-by: Patrick Georgi <[email protected]> > I didn't review the code in depth, so my comments may be incomplete. > Did you write those functions from scratch, or did you merge proven code > from a BSD licensed codebase? I adapted libpayload functions where possible (strtoul -> strtol, for example). Everything else is written from scratch. > strtol is long int, not int. I'll change that for both strtol and strtoul, thanks. >> +char *strcat(char *d, const char *s) > What about more descriptive argument names? I adopted naming style (ie. single character names) from the other functions (strncat for strcat, specifically). This should be done globally, in a separate patch, in my opinion. >> Index: include/pci.h > Here is seems you're using both spaces and tabs between the identifier > and the value. Apart from that, I found that some places use spaces for > indentation instead of tabs. Fixed. >> +int getpagesize(void) >> +{ >> + return 4096; >> > > Maybe wrap that in a check for x86. OTOH, if this function is used to > determine mmap granularity, it should return 1 instead of 4096. I stuck with the meaning of the function (and x86 has 4k pages by default). Instead of #ifdef-hell, I moved the function to arch/*/virtual.c - both x86 and powerpc claim 4k pages now. Patrick -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
