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

[Albert ARIBAUD]

Le 08/11/2011 16:57, Simon Glass a écrit :
> 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.

I don't think 'ifdefs are' necessarily 'a pain'. They cater for a need, 
that is, to mark that some code depends on some condition. I find it 
*normal* that a checksum-related variable is conditioned on the checksum 
macro being defined.

> Regards,
> Simon

Amicalement,
-- 
Albert.