* [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c @ 2014-10-12 10:25 Hans de Goede 2014-10-12 10:25 ` Hans de Goede 2014-10-12 16:13 ` Jeroen Hofstee 0 siblings, 2 replies; 6+ messages in thread From: Hans de Goede @ 2014-10-12 10:25 UTC (permalink / raw) To: u-boot Hi, This one seems to have fallen through the cracks. Regards, Hans ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c 2014-10-12 10:25 [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c Hans de Goede @ 2014-10-12 10:25 ` Hans de Goede 2014-10-12 16:13 ` Jeroen Hofstee 1 sibling, 0 replies; 6+ messages in thread From: Hans de Goede @ 2014-10-12 10:25 UTC (permalink / raw) To: u-boot This fixes the following compiler warning: In file included from scripts/kconfig/zconf.tab.c:2537:0: scripts/kconfig/menu.c: In function ?get_symbol_str?: scripts/kconfig/menu.c:590:18: warning: ?jump? may be used uninitialized in this function [-Wmaybe-uninitialized] jump->offset = strlen(r->s); ^ In file included from scripts/kconfig/zconf.tab.c:2537:0: scripts/kconfig/menu.c:551:19: note: ?jump? was declared here struct jump_key *jump; Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- scripts/kconfig/menu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index a26cc5d..584e0fc 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -548,7 +548,7 @@ static void get_prompt_str(struct gstr *r, struct property *prop, { int i, j; struct menu *submenu[8], *menu, *location = NULL; - struct jump_key *jump; + struct jump_key *jump = NULL; str_printf(r, _("Prompt: %s\n"), _(prop->text)); menu = prop->menu->parent; -- 2.1.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c 2014-10-12 10:25 [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c Hans de Goede 2014-10-12 10:25 ` Hans de Goede @ 2014-10-12 16:13 ` Jeroen Hofstee 2014-10-13 5:14 ` Simon Glass 1 sibling, 1 reply; 6+ messages in thread From: Jeroen Hofstee @ 2014-10-12 16:13 UTC (permalink / raw) To: u-boot 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. Regards, Jeroen ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c 2014-10-12 16:13 ` Jeroen Hofstee @ 2014-10-13 5:14 ` Simon Glass 2014-10-13 6:48 ` Jeroen Hofstee 0 siblings, 1 reply; 6+ messages in thread From: Simon Glass @ 2014-10-13 5:14 UTC (permalink / raw) To: u-boot 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? Regards, Simon ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c 2014-10-13 5:14 ` Simon Glass @ 2014-10-13 6:48 ` Jeroen Hofstee 2014-10-13 9:01 ` Tom Rini 0 siblings, 1 reply; 6+ messages in thread From: Jeroen Hofstee @ 2014-10-13 6:48 UTC (permalink / raw) To: u-boot 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? Regards, Jeroen ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c 2014-10-13 6:48 ` Jeroen Hofstee @ 2014-10-13 9:01 ` Tom Rini 0 siblings, 0 replies; 6+ messages in thread From: Tom Rini @ 2014-10-13 9:01 UTC (permalink / raw) To: u-boot 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141013/7922ed89/attachment.pgp> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-13 9:01 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-10-12 10:25 [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c Hans de Goede 2014-10-12 10:25 ` Hans de Goede 2014-10-12 16:13 ` Jeroen Hofstee 2014-10-13 5:14 ` Simon Glass 2014-10-13 6:48 ` Jeroen Hofstee 2014-10-13 9:01 ` Tom Rini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox