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); +

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

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));

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

On 01/11/2018 10:02 AM, Marc Hartmayer wrote: > 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. > Let's see what Michal comes up with in his vsh series, see: https://www.redhat.com/archives/libvir-list/2018-January/msg00365.html I had seen his series, so I hesitated in pushing this until I (or someone else) had a chance to look at his series and determine if he addressed it at all. It's not like the assert issue is recent, so hopefully we can wait to see what he comes up with.

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