From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Tue, 16 Apr 2013 20:06:35 +0200
Subject: [U-Boot] [PATCH v3 8/8] palmtreo680: add utility that writes
	u-boot to flash
In-Reply-To: <516D8F75.6030404@newsguy.com>
References: <1365793160-18247-1-git-send-email-mikedunn@newsguy.com>
	<201304141938.18234.marex@denx.de> <516D8F75.6030404@newsguy.com>
Message-ID: <201304162006.35808.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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