[libvirt] [PATCH] libvirt-domain: Drop virDomainMigrateCheckNotLocal attribute
Our apibuild.py script does not cope with ATTRIBUTE_NONNULL: Parse Error: parsing function type, ')' expected Got token ('name', 'char') Last token: ('name', 'char') Token queue: [('op', '*'), ('name', 'dconnuri'), ('sep', ')')] Line 3297 end: Makefile:2441: recipe for target '../../docs/apibuild.py.stamp' failed Let's drop it. Moreover, up until e17ae3ccc2dbc1400 where it was introduced we did not really care about NULL-ity of dconnuri. And moreover the ATTRIBUTE_NONNULL merely checks for static calls over NULL, it won't catch the dynamic ones, where a NULL is passed by a variable at runtime. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index e6c8604..6e1aacd 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3293,7 +3293,7 @@ virDomainMigrateVersion3Params(virDomainPtr domain, } -static int ATTRIBUTE_NONNULL(1) +static int virDomainMigrateCheckNotLocal(const char *dconnuri) { virURIPtr tempuri = NULL; -- 2.4.9
On Mon, Oct 12, 2015 at 16:41:49 +0200, Michal Privoznik wrote:
Our apibuild.py script does not cope with ATTRIBUTE_NONNULL:
Parse Error: parsing function type, ')' expected Got token ('name', 'char') Last token: ('name', 'char') Token queue: [('op', '*'), ('name', 'dconnuri'), ('sep', ')')] Line 3297 end: Makefile:2441: recipe for target '../../docs/apibuild.py.stamp' failed
Let's drop it. Moreover, up until e17ae3ccc2dbc1400 where it was introduced we did not really care about NULL-ity of dconnuri. And moreover the ATTRIBUTE_NONNULL merely checks for static calls over NULL, it won't catch the dynamic ones, where a NULL is passed by a variable at runtime.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index e6c8604..6e1aacd 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3293,7 +3293,7 @@ virDomainMigrateVersion3Params(virDomainPtr domain, }
-static int ATTRIBUTE_NONNULL(1) +static int virDomainMigrateCheckNotLocal(const char *dconnuri) { virURIPtr tempuri = NULL;
ACK Jirka
On 10/12/2015 10:41 AM, Michal Privoznik wrote:
Our apibuild.py script does not cope with ATTRIBUTE_NONNULL:
Parse Error: parsing function type, ')' expected Got token ('name', 'char') Last token: ('name', 'char') Token queue: [('op', '*'), ('name', 'dconnuri'), ('sep', ')')] Line 3297 end: Makefile:2441: recipe for target '../../docs/apibuild.py.stamp' failed
Let's drop it. Moreover, up until e17ae3ccc2dbc1400 where it was introduced we did not really care about NULL-ity of dconnuri. And moreover the ATTRIBUTE_NONNULL merely checks for static calls over NULL, it won't catch the dynamic ones, where a NULL is passed by a variable at runtime.
ACK. (I have a fairly low opinion of ATTRIBUTE_NONNULL - at first glance it *appears* that it should be doing something to guarantee that passed args are non-null, but instead it is really just a hint to the compiler's optimizer (and to static checkers like coverity) that the programmer *thinks* there are no instances of NULL being passed in this argument. This can result in useful checks (at runtime and during a coverity run) being skipped. I don't remember the details, but have a vague memory of a bug I was having trouble tracking down, and it ended up being due to an ATTRIBUTE_NONNULL placed on an arg that *wasn't* always non-NULL.)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index e6c8604..6e1aacd 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3293,7 +3293,7 @@ virDomainMigrateVersion3Params(virDomainPtr domain, }
-static int ATTRIBUTE_NONNULL(1) +static int virDomainMigrateCheckNotLocal(const char *dconnuri) { virURIPtr tempuri = NULL;
On 12.10.2015 17:06, Laine Stump wrote:
On 10/12/2015 10:41 AM, Michal Privoznik wrote:
Our apibuild.py script does not cope with ATTRIBUTE_NONNULL:
Parse Error: parsing function type, ')' expected Got token ('name', 'char') Last token: ('name', 'char') Token queue: [('op', '*'), ('name', 'dconnuri'), ('sep', ')')] Line 3297 end: Makefile:2441: recipe for target '../../docs/apibuild.py.stamp' failed
Let's drop it. Moreover, up until e17ae3ccc2dbc1400 where it was introduced we did not really care about NULL-ity of dconnuri. And moreover the ATTRIBUTE_NONNULL merely checks for static calls over NULL, it won't catch the dynamic ones, where a NULL is passed by a variable at runtime.
ACK.
(I have a fairly low opinion of ATTRIBUTE_NONNULL - at first glance it *appears* that it should be doing something to guarantee that passed args are non-null, but instead it is really just a hint to the compiler's optimizer (and to static checkers like coverity) that the programmer *thinks* there are no instances of NULL being passed in this argument. This can result in useful checks (at runtime and during a coverity run) being skipped. I don't remember the details, but have a vague memory of a bug I was having trouble tracking down, and it ended up being due to an ATTRIBUTE_NONNULL placed on an arg that *wasn't* always non-NULL.)
Yes, that was my recollection too when writing the commit message, but was just too lazy to dig out the commit you're talking about. Nevertheless, ATTRIBUTE_NONNULL over an attribute will make compiler skip check for NULL, i.e. if (!var) reportError(); will be totally dropped during compilation. Anyway, pushed. Thanks! Michal
On Mon, Oct 12, 2015 at 17:19:18 +0200, Michal Privoznik wrote:
On 12.10.2015 17:06, Laine Stump wrote:
On 10/12/2015 10:41 AM, Michal Privoznik wrote:
...
(I have a fairly low opinion of ATTRIBUTE_NONNULL - at first glance it *appears* that it should be doing something to guarantee that passed args are non-null, but instead it is really just a hint to the compiler's optimizer (and to static checkers like coverity) that the programmer *thinks* there are no instances of NULL being passed in this argument. This can result in useful checks (at runtime and during a coverity run) being skipped. I don't remember the details, but have a vague memory of a bug I was having trouble tracking down, and it ended up being due to an ATTRIBUTE_NONNULL placed on an arg that *wasn't* always non-NULL.)
Yes, that was my recollection too when writing the commit message, but was just too lazy to dig out the commit you're talking about. Nevertheless, ATTRIBUTE_NONNULL over an attribute will make compiler skip check for NULL, i.e. if (!var) reportError(); will be totally dropped during compilation.
That's the exact reason why ATTRIBUTE_NONNULL was macroed out as a no-op if you don't do static analysis builds. Peter
participants (4)
-
Jiri Denemark -
Laine Stump -
Michal Privoznik -
Peter Krempa