[U-Boot] [PATCH V3 3/4] ARM: Warn when the machine ID isn't set.

Albert ARIBAUD albert.u.boot at aribaud.net
Sun Jul 17 11:00:47 CEST 2011


Hi all,

Le 17/07/2011 10:26, stefano babic a écrit :
> Am 17/07/2011 08:53, schrieb Igor Grinberg:
>>>> Also, in the printf line, you are mixing tabs with spaces
>>>> (sorry for not noticing this in previous versions...).
>>> ...and if you want to print something only for debug purposes, the best
>>> way is to substitute printf() with debug() and get rid of #ifdef.
>>>
>>> +	if (machid == BI_ARCH_NUMBER_INVALID)
>>> +	        debug("Warning: machid not set.\n");
>>
>> That is understood completely and that is not what I'm asking...
>> I think that this warning should be printed not just for debug purposes...
>> So, I'd prefer:
>>
>> +	if (machid == BI_ARCH_NUMBER_INVALID)
>> +		printf("Warning: machid not set.\n");
>>
>> with no #ifdefs.
>
> Agree. And because the goal of thi patch is to warn when the mach-id is
> not set, I am expecting to see this warning on the console without
> recompiling the code.
>
>> So, I'm asking is it essential to make it only for debug purposes?
>
> IMHO, I think no.
>
>> Are there any cases when this code will harm if no #define DEBUG is specified?
>
> Agree with you, I do not see any reason to output the warning only if
> DEBUG is set

Agreed as well, only more so: I see reason for this warning *only* if it 
is emitted in non-DEBUG builds. Such a warning is emitted to warn 
against possible future complications; if it is only emitted in DEBUG 
builds, then when the complication actually happens, that is, in a 
production build, the developer is deprived of an important clue 
regarding the cause.

> Best regards,
> Stefano Babic

Amicalement,
-- 
Albert.


More information about the U-Boot mailing list