[U-Boot] [PATCH v3 8/8] palmtreo680: add utility that writes u-boot to flash
Marek Vasut
marex at denx.de
Tue Apr 16 20:06:35 CEST 2013
Dear Mike Dunn,
> Thanks again Marek. A question below...
>
>
> On 04/14/2013 10:38 AM, Marek Vasut wrote:
>
>
> [...]
>
> >> +
> >> + if (argc != 3) {
> >> + printf("usage: %s <image file> <mtd dev node>\n", argv[0]);
> >> + exit(-1);
> >
> > Use proper errno and "return" as you're returning from main() anyway.
>
> Agreed regarding 'return'. But should I be concerned with setting or
> preserving errno before all 'return -1' lines? Is it normal practice for
> a common utility to set errno? errno will have to be saved in many
> places, since perror() itself can change it. This will add many more
> lines of code.
Ooops! errno.h, sorry for the confusion :-(
> [...]
>
> >> +
> >> + blockbuf = malloc(RELIABLE_BLOCKSIZE);
> >
> > Do you not want to use some calloc() here to make sure the "blockbuf" is
> > zeroed?
>
> Not necessary here; the buffer is always filled or the utility exits with
> error. But will change to calloc() anyway.
If you're sure it's filled, then it's no problem.
> [...]
>
> >> +
> >> + /* read data for one block from file */
> >> + while (len != 0 && (read_ret = read(datafd, buf, len)) != 0) {
> >
> > Uh, this really might be a candidate for IOCCC, split this please ...
>
> Well, OK, but... I normally don't embed calls in tests, but I do it here
> because the read is performed at the start of each loop iteration, and I
> thought this made it clearer and more concise. Basically it means "loop
> while there's still more data to write, and read() does not return EOF".
>
> Actually, read() should never return EOF, because earlier I check the file
> length, so if I'm going to do the sanity check anyway, maybe it should be
> separate.
I'd say you can loop and break; out if needed, no ?
while (cond.) {
if (x)
break;
if (y)
break;
do_useful_stuff here;
}
Best regards,
Marek Vasut
More information about the U-Boot
mailing list