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

[Simon Glass]

Hi Detlev,

On Tue, Nov 8, 2011 at 1:20 AM, Detlev Zundel <dzu at denx.de> wrote:
> Hi Mike,
>
>> 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.
>
> Personally, I think that the nuisance of a potential unused variable is
> less of an issue than the actual _problems_ that ifdefs induce.

Yes the #ifdefs are a pain. I am working on a replacement for board.c
- so far I have split things into functions. Next I need to look at
Graeme's initcall patch.
..
Regards,
Simon

>
> Cheers
>  Detlev
>
> --
> The best way to predict the future is to invent it.
>                                           -- Alan Kay
> --
> DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de
>