Re: [libvirt] [PATCH] vsh: add a necessary assertion

On Thu, Jan 04, 2018 at 08:20 PM +0100, John Ferlan <jferlan(a)redhat.com&gt; wrote: > On 12/18/2017 07:33 AM, Marc Hartmayer wrote: >> This fixes the compilation error (compiled with the compiler option >> '-03'). >> >> In file included from ../../tools/vsh.c:28:0: >> ../../tools/vsh.c: In function 'vshCommandOptStringQuiet': >> ../../tools/vsh.c:838:30: error: potential null pointer dereference [-Werror=null-dereference] >> assert(!needData || valid->type != VSH_OT_BOOL); >> >> Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com&gt; >> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; >> --- >> tools/vsh.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tools/vsh.c b/tools/vsh.c >> index e878119b988f..677eb9db3e41 100644 >> --- a/tools/vsh.c >> +++ b/tools/vsh.c >> @@ -816,8 +816,8 @@ vshCommandFree(vshCmd *cmd) >> * to the option if found, 0 with *OPT set to NULL if the name is >> * valid and the option is not required, -1 with *OPT set to NULL if >> * the option is required but not present, and assert if NAME is not >> - * valid (which indicates a programming error). No error messages are >> - * issued if a value is returned. >> + * valid or the option was not found (which indicates a programming >> + * error). No error messages are issued if a value is returned. >> */ >> static int >> vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt, >> @@ -835,6 +835,8 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt, >> break; >> valid++; >> } >> + assert(valid); >> + > > Could we combine the subsequent one to have : > > assert(valid && (!needData || valid->type != VSH_OT_BOOL)); Shall I resend a v2? Thanks.

> > ? > > John > >> assert(!needData || valid->type != VSH_OT_BOOL); >> if (valid->flags & VSH_OFLAG_REQ) >> ret = -1; >> >