Hi Jeroen
Hello Albert,
On 21-11-14 16:30, Albert ARIBAUD wrote:
On Fri, 21 Nov 2014 13:34:41 +0100, Jeroen Hofstee jeroen@myspectrum.nl wrote:
But oh well, if it fixes a warning :-)
I didn't claim that there is a bug in the code :-).
I just get annoying when on my continuous integration script I see the same warning for all cross compiled boards.
Wouldn't it be better to simply disable the -Wmaybe-uninitialized for gcc?
Disabling a warning is hiding potential dust under the carpet IMO
Agreed in general, but not for this one, since "fixing" is the carpet,
I assume that you are presenting below an answer to a "general" case.
However, as Thomas pointed out earlier, this "fix" is perfectly safe regarding the underlying kwbimage code.
as far a I can tell. This is roughly the case which causes the warning e.g. (and variant like this with a switch, etc):
char *a;
if (something) a = something_valid
[...]
if (something) *a = 1;
Some gcc versions start complaining about the second instance, that it _might_ be used uninitialized.
With the "fix" this will no longer warn:
char *a = 0; /* not valid, just set to stop gcc from complaining */
*a = 1; // paved away _error_, to suppress an invalid warning..
if (something) a = something_valid
....
if (something) *a = 1;
Since 0 is a perfectly valid address in u-boot it should emit no warning whatsoever, just crash at runtime.
I got your point.
and the only justification I see as acceptable for doing so is when leaving the warning enabled would cause an obnoxiously high number of false positives.
Well let me add, if "fixing the warning" causes real error to be hidden, we shouldn't "fix" the warnings by modifying valid code.
Each subsequent "fix" for this kind of warning should be considered case by case IMHO, therefore I agree with Albert.
Regards, Jeroen
Best regards, Lukasz Majewski