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@xxxxxxxxxxxxx> 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:
signature.asc
Description: Digital signature
