[U-Boot] [PATCH] fix: tools: kwbimage.c: Initialize headersz to suppress warning

Lukasz Majewski l.majewski at majess.pl
Sat Nov 22 07:56:35 CET 2014


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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141122/2b42f65d/attachment.pgp>


More information about the U-Boot mailing list