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

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(a)redhat.com&gt; > > --- > > 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].