diff mbox

[U-Boot,04/13] common/cmd_pxe.c: Fix compile warning

Message ID 1323429272-26801-5-git-send-email-wd@denx.de
State Superseded
Headers show

Commit Message

Wolfgang Denk Dec. 9, 2011, 11:14 a.m. UTC
Fix:
cmd_pxe.c: In function 'parse_pxefile_top':
cmd_pxe.c:941:5: warning: 'err' may be used uninitialized in this
function [-Wuninitialized]
cmd_pxe.c:921:6: note: 'err' was declared here

Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Jason Hobbs <jason.hobbs@calxeda.com>
---
 common/cmd_pxe.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Comments

Jason Hobbs Dec. 9, 2011, 1:48 p.m. UTC | #1
Dear Wolfgang,

On Fri, Dec 09, 2011 at 06:14:23AM -0500, Wolfgang Denk wrote:
> Fix:
> cmd_pxe.c: In function 'parse_pxefile_top':
> cmd_pxe.c:941:5: warning: 'err' may be used uninitialized in this
> function [-Wuninitialized]
> cmd_pxe.c:921:6: note: 'err' was declared here
> 
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Jason Hobbs <jason.hobbs@calxeda.com>
> ---
>  common/cmd_pxe.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
> index 9426f5b..eaf95bf 100644
> --- a/common/cmd_pxe.c
> +++ b/common/cmd_pxe.c
> @@ -936,6 +936,7 @@ static int parse_menu(char **c, struct pxe_menu *cfg, char *b, int nest_level)
>  	default:
>  		printf("Ignoring malformed menu command: %.*s\n",
>  				(int)(*c - s), s);
> +		err = -1;

err should either be set to 0 here, or initialized to 0 at the top of
the function. Setting it to -1 will cause the parser to give up rather
than just printing out the warning message. It doesn't have to give up,
and not giving up makes the parser more accommodating of pxelinux
commands that aren't supported in U-Boot.

Thanks,
Jason
Wolfgang Denk Dec. 9, 2011, 8:45 p.m. UTC | #2
Dear Jason,

In message <20111209134819.GA26840@jhobbs-laptop> you wrote:
> 
> >  	default:
> >  		printf("Ignoring malformed menu command: %.*s\n",
> >  				(int)(*c - s), s);
> > +		err = -1;
> 
> err should either be set to 0 here, or initialized to 0 at the top of
> the function. Setting it to -1 will cause the parser to give up rather
> than just printing out the warning message. It doesn't have to give up,
> and not giving up makes the parser more accommodating of pxelinux
> commands that aren't supported in U-Boot.

You have way more experience with PXE than me, but if we runinto this
case, doesn't that mean that the whole menu setup is severely broken,
and continuing is more or less invoking random behaviour?

If you really want to see a 0 here, then please feel free to submit an
updated / fixed patch.

Thanks.

Best regards,

Wolfgang Denk
Jason Hobbs Dec. 13, 2011, 12:50 p.m. UTC | #3
On Fri, Dec 09, 2011 at 03:45:44PM -0500, Wolfgang Denk wrote:
> Dear Jason,
> 
> In message <20111209134819.GA26840@jhobbs-laptop> you wrote:
> > 
> > >  	default:
> > >  		printf("Ignoring malformed menu command: %.*s\n",
> > >  				(int)(*c - s), s);
> > > +		err = -1;
> > 
> > err should either be set to 0 here, or initialized to 0 at the top of
> > the function. Setting it to -1 will cause the parser to give up rather
> > than just printing out the warning message. It doesn't have to give up,
> > and not giving up makes the parser more accommodating of pxelinux
> > commands that aren't supported in U-Boot.
> 
> You have way more experience with PXE than me, but if we runinto this
> case, doesn't that mean that the whole menu setup is severely broken,
> and continuing is more or less invoking random behaviour?

It only means that an unrecognized menu command was used. It could be
something aesthetic in nature, like a menu title command.

> 
> If you really want to see a 0 here, then please feel free to submit an
> updated / fixed patch.

Heiko Schocher ended up sending a patch to do this today, which I've
acked.

Thanks,

Jason
diff mbox

Patch

diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
index 9426f5b..eaf95bf 100644
--- a/common/cmd_pxe.c
+++ b/common/cmd_pxe.c
@@ -936,6 +936,7 @@  static int parse_menu(char **c, struct pxe_menu *cfg, char *b, int nest_level)
 	default:
 		printf("Ignoring malformed menu command: %.*s\n",
 				(int)(*c - s), s);
+		err = -1;
 	}
 
 	if (err < 0)