Hi Mike, On Wed, Mar 14, 2012 at 10:24 PM, Mike Frysinger <vap...@gentoo.org> wrote: > On Thursday 15 March 2012 00:09:59 Simon Glass wrote: >> On Mon, Mar 12, 2012 at 8:22 AM, Mike Frysinger wrote: >> > +/* Used to quickly bulk erase backing store */ >> > +static u8 sb_sf_0xff[0x10000]; >> >> Ick, Does it really need to be so large? > > in order to do a single write() for a single sector, yes. it also simplifies > the code as i don't have to loop over some smaller size and keep track of how > many bytes i've written until a sector is cleared. > > this is in the bss and it's 64KiB. i'm really not worried about your > development system running Chrome getting low on RAM :).
No, it's more that I think it is a bit grubby. But OK. > >> > + sbsf->fd = os_open(file, 02); >> > + if (sbsf->fd == -1) { >> > + free(sbsf); >> > + goto error; >> >> Prints incorrect error if the file couldn't be found/open. I wonder if >> we should have os_perror() ? > > probably would be useful. we prob have to document that while os_perror() > would work, strerror(errno) would not. the os_* funcs update the errno that > the C library knows about (or *__errno_location() in glibc), but u-boot > declares a local "errno" variable. so attempting to reference "errno" in u- > boot code will always resolve to the local definition rather than the glibc > one. os_perror() would work because it calls __errno_location() internally > just like the other os_* funcs. Yes a bit messy, let's leave it for now then. > >> > + pos += os_read(sbsf->fd, tx + pos, cnt); >> >> This can fail (return -1) > > the ways in which read/write could fail aren't really possible for us, but i > guess it won't hurt to assert() the values. Are you sure? What if the file has a read error? That could happen in the test setup. To me an assert is a bit ugly for this sort of thing. Also for test code we don't want it dying in the bowels of the test with an assert - better to return test failure so people can see what went wrong. Regards, Simon > -mike _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot