Re: [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c

[Tom Rini <trini@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2405 bytes --]

On Mon, Oct 13, 2014 at 08:48:39AM +0200, Jeroen Hofstee wrote:
> Hello Simon,
> 
> On 13-10-14 07:14, Simon Glass wrote:
> >Hi Jeroen,
> >
> >On 12 October 2014 10:13, Jeroen Hofstee <jeroen@myspectrum.nl> wrote:
> >
> >>Hello Hans,
> >>
> >>On 12-10-14 12:25, Hans de Goede wrote:
> >>
> >>>Hi,
> >>>
> >>>This one seems to have fallen through the cracks.
> >>>
> >>>Regards,
> >>>
> >>>Hans
> >>>
> >>>  (for U-boot)
> >>nope, you replace an innocent warning (_might_ be) with
> >>bad code, without any comment it is just because gcc failed
> >>to recognize it is fine. Nor did you respond to the suggestion
> >>if it helps gcc to recognize that if the two booleans are merged
> >>into a single one. [or even split it in an if () if ()]. With this patch
> >>you prevent any serious warning in case the variable is actually
> >>used but not initialized, which is even worse if you ask me.
> >>
> >That is a pretty acerbic tone to take on the U-Boot list at least. Are you
> >two drinking buddies or something?
> 
> no, it is because we have discussed this patch before and resending
> it won't address the issue raised. But you are right, it is likely done with
> less evil intends then I took it for, so let me explain my concern again
> in a politer way. The problem is that gcc 4.9 starts warning in the
> following case:
> 
> int *ptr;
> 
> if (a)
>     ptr = something;
> 
> if (a && b)
>     ptr->bla = value;
> else
>    do_something_else();
> 
> 
> it will warn that ptr _might_ be used uninitialized (but it always is).
> This is fixed in this patch by assigning NULL to ptr, and while that makes
> the warning go away it actually prevents the valid warning, ptr _is_ used
> uninitialized if you start using it in the else case. Hence my request if we
> can't find a better solution for this.
> 
> Does anyone know a better solution for this or should we consider
> disabling the might be unused warning?

Frankly, looking at the code, this is a compiler bug since as you note
the pointer will always be initalized.  Since we share this code as-is
with upstream kernel, we should see if there's any interst there in
trying to re-write the code so that it's (roughly):
if (a)
 ptr = valid;

if (a && b && ptr)
 ptr->foo = bar;

Or if this gets the required "compiler is being stupid, file a bug"
volume required.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]