[U-Boot] [PATCH v2] arm: Correct build error introduced by getenv_ulong() patch

[Mike Frysinger]

On Monday 31 October 2011 17:06:46 Simon Glass wrote:
> On Sun, Oct 30, 2011 at 5:44 PM, Mike Frysinger wrote:
> > On Sunday 23 October 2011 23:44:35 Simon Glass wrote:
> >> --- a/arch/arm/lib/board.c
> >> +++ b/arch/arm/lib/board.c
> >> 
> >>       flash_size = flash_init();
> >>       if (flash_size > 0) {
> >>  # ifdef CONFIG_SYS_FLASH_CHECKSUM
> >> +             char *s = getenv("flashchecksum");
> >> +
> >>               print_size(flash_size, "");
> >>               /*
> >>                * Compute and print flash CRC if flashchecksum is set to
> >> 'y' *
> >>                * NOTE: Maybe we should add some WATCHDOG_RESET()? XXX
> >>                */
> >> -             s = getenv("flashchecksum");
> >>               if (s && (*s == 'y')) {
> >>                       printf("  CRC: %08X", crc32(0,
> >>                               (const unsigned char *)
> >> CONFIG_SYS_FLASH_BASE, @@ -566,9 +567,12 @@ void board_init_r(gd_t *id,
> >> ulong dest_addr) /* Initialize from environment */
> >>       load_addr = getenv_ulong("loadaddr", 16, load_addr);
> >>  #if defined(CONFIG_CMD_NET)
> >> -     s = getenv("bootfile");
> >> -     if (s != NULL)
> >> -             copy_filename(BootFile, s, sizeof(BootFile));
> >> +     {
> >> +             char *s = getenv("bootfile");
> >> +
> >> +             if (s != NULL)
> >> +                     copy_filename(BootFile, s, sizeof(BootFile));
> >> +     }
> >>  #endif
> > 
> > seems like a better solution would be to use at the top:
> >        __maybe_unused char *s;
> > 
> > also, shouldn't these be "const char *s" ?
> 
> We can certainly do this and I agree it is easier than #ifdefs. Does
> it introduce the possibility that one day the code will stop using the
> variable but it will still be declared? Is the fact that we need the
> #ifdefs an indication that the function should be too long and should
> be refactored? it in fact better to have these explicit so we can see
> them for the ugliness they are?

yes, you're right that it does leave the door open to the variable being 
declared, never used, and gcc not emitting a warning about it.

both setups suck, but i'd lean towards the less-ifdef state ... wonder if 
Wolfgang has a preference.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111031/948e5d8d/attachment.pgp