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

Lukasz Majewski
2014-11-22T06:56:35Z

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
-------------- 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>