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

2014-10-13 Thread Jeroen Hofstee

Hello Simon,

On 13-10-14 07:14, Simon Glass wrote:

Hi Jeroen,

On 12 October 2014 10:13, Jeroen Hofstee jer...@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

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-10-13 Thread Tom Rini
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 jer...@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


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-10-12 Thread Hans de Goede
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 hdego...@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

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-10-12 Thread Hans de Goede
Hi,

This one seems to have fallen through the cracks.

Regards,

Hans
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-10-12 Thread Jeroen Hofstee

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


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-10-12 Thread Simon Glass
Hi Jeroen,

On 12 October 2014 10:13, Jeroen Hofstee jer...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot