[U-Boot] [PATCH v3 8/8] palmtreo680: add utility that writes u-boot to flash

[Mike Dunn <mikedunn@newsguy.com>]

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.


[...]


>> +
>> +	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.

[...]


>> +
>> +		/* 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.

Thanks,
Mike