Hi Albert,
On Tue, Nov 8, 2011 at 11:46 AM, Albert ARIBAUD
<albert.u.b...@aribaud.net> wrote:
> Le 08/11/2011 16:57, Simon Glass a écrit :
>>
>> Hi Detlev,
>>
>> On Tue, Nov 8, 2011 at 1:20 AM, Detlev Zundel<d...@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.
>
This discussion was regarding the need to #ifdef the variable declaration, viz:
#if defined(THING1) || defined(THING2)
const char *cat;
#endif
...
#ifdef THING1
cat = getenv("cat");
send_back(cat);
#endif
....
#ifdef THING2
cat = check_outside("cat");
if (cat)
wibble(cat);
#endif
and whether the top bit would be better as:
__maybe_unused const char *cat;
But more generally, lots of #ifdefs do make the code harder to read,
and potentially more brittle in the face of config changes.
Regards,
Simon
>
> Amicalement,
> --
> Albert.
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
