[libvirt PATCH 10/11] vircommand: Remove NULL check in virCommandAddArg

[Tim Wiederhake]

On Thu, 2021-01-28 at 11:54 +0100, Peter Krempa wrote:
> On Thu, Jan 28, 2021 at 11:24:40 +0100, Tim Wiederhake wrote:
> > `val` is declared `ATTRIBUTE_NONNULL`.
> 
> Please see:
> 
> https://gitlab.com/libvirt/libvirt/-/blob/master/src/internal.h#L127
> 
> ATTRIBUTE_NONNULL is unfortunately lead to wrong optimizations done
> by
> gcc.
> 
> > Found by clang-tidy's "clang-diagnostic-tautological-pointer-
> > compare"
> > check.
> > 
> > Signed-off-by: Tim Wiederhake <twiederh at redhat.com>
> > ---
> >  src/util/vircommand.c | 5 -----
> >  1 file changed, 5 deletions(-)
> > 
> > diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> > index c3a98bbeac..223010c6aa 100644
> > --- a/src/util/vircommand.c
> > +++ b/src/util/vircommand.c
> > @@ -1525,11 +1525,6 @@ virCommandAddArg(virCommandPtr cmd, const
> > char *val)
> >      if (!cmd || cmd->has_error)
> >          return;
> >  
> > -    if (val == NULL) {
> > -        cmd->has_error = EINVAL;
> > -        return;
> > -    }
> 
> So this might have actual value.
> 

Hm, I see. I will remove this patch from the series for now. A bit
frustrating though, if you compare with, as a random example,
"virCommandGetGID", which is declared in [1] as "ATTRIBUTE_NONNULL" for
argument "cmd", which is subsequently dereferenced without additional
NULL check in [2].

Cheers,
Tim

[1] 
https://gitlab.com/libvirt/libvirt/-/blob/master/src/util/vircommand.h#L71
[2] 
https://gitlab.com/libvirt/libvirt/-/blob/master/src/util/vircommand.c#L1116