bugifx: fix some wrong usage of ATTRIBUTE_NONNULL

Bihong Yu posted 1 patch 3 years, 10 months ago
Failed in applying to current master (apply log)
src/libvirt-domain.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
bugifx: fix some wrong usage of ATTRIBUTE_NONNULL
Posted by Bihong Yu 3 years, 10 months ago
There are some wrong usage of ATTRIBUTE_NONNULL, which may cause the compilation fail. The
ATTRIBUTE_NONNULL is the define of __attribute__((__nonnull__(m))), which declares that the
input pointer parameter of funciton should not be NULL. If we declare some input pointer
parameter of the function is ATTRIBUTE_NONNULL, the function should not redundancy check of
the pointer parameter. And the ATTRIBUTE_NONNULL can only be using to pointer.

>From 55cd85345b2dc50f44c1e382563482d40142382b Mon Sep 17 00:00:00 2001
From: yubihong <yubihong@huawei.com>
Date: Fri, 24 Apr 2020 17:44:43 +0800
Subject: [PATCH] qemu: fix code format problem

Signed-off-by:Yu Bihong <yubihong@huawei.com>
---
 src/libvirt-domain.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index a12809c..d659f1b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -8194,11 +8194,11 @@ virDomainAttachDevice(virDomainPtr domain, const char *xml)
     virCheckReadOnlyGoto(conn->flags, error);

     if (conn->driver->domainAttachDevice) {
-       int ret;
-       ret = conn->driver->domainAttachDevice(domain, xml);
-       if (ret < 0)
-          goto error;
-       return ret;
+        int ret;
+        ret = conn->driver->domainAttachDevice(domain, xml);
+        if (ret < 0)
+            goto error;
+        return ret;
     }

     virReportUnsupportedError();
@@ -8299,9 +8299,9 @@ virDomainDetachDevice(virDomainPtr domain, const char *xml)
     if (conn->driver->domainDetachDevice) {
         int ret;
         ret = conn->driver->domainDetachDevice(domain, xml);
-         if (ret < 0)
-             goto error;
-         return ret;
+        if (ret < 0)
+            goto error;
+        return ret;
      }

     virReportUnsupportedError();
-- 
1.8.3.1
From 55cd85345b2dc50f44c1e382563482d40142382b Mon Sep 17 00:00:00 2001
From: yubihong <yubihong@huawei.com>
Date: Fri, 24 Apr 2020 17:44:43 +0800
Subject: [PATCH] qemu: fix code format problem

Signed-off-by:Yu Bihong <yubihong@huawei.com>
---
 src/libvirt-domain.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index a12809c..d659f1b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -8194,11 +8194,11 @@ virDomainAttachDevice(virDomainPtr domain, const char *xml)
     virCheckReadOnlyGoto(conn->flags, error);
 
     if (conn->driver->domainAttachDevice) {
-       int ret;
-       ret = conn->driver->domainAttachDevice(domain, xml);
-       if (ret < 0)
-          goto error;
-       return ret;
+        int ret;
+        ret = conn->driver->domainAttachDevice(domain, xml);
+        if (ret < 0)
+            goto error;
+        return ret;
     }
 
     virReportUnsupportedError();
@@ -8299,9 +8299,9 @@ virDomainDetachDevice(virDomainPtr domain, const char *xml)
     if (conn->driver->domainDetachDevice) {
         int ret;
         ret = conn->driver->domainDetachDevice(domain, xml);
-         if (ret < 0)
-             goto error;
-         return ret;
+        if (ret < 0)
+            goto error;
+        return ret;
      }
 
     virReportUnsupportedError();
-- 
1.8.3.1

Re: bugifx: fix some wrong usage of ATTRIBUTE_NONNULL
Posted by Daniel Henrique Barboza 3 years, 10 months ago
For the attached patch:


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

On 6/6/20 7:30 AM, Bihong Yu wrote:
> There are some wrong usage of ATTRIBUTE_NONNULL, which may cause the compilation fail. The
> ATTRIBUTE_NONNULL is the define of __attribute__((__nonnull__(m))), which declares that the
> input pointer parameter of funciton should not be NULL. If we declare some input pointer
> parameter of the function is ATTRIBUTE_NONNULL, the function should not redundancy check of
> the pointer parameter. And the ATTRIBUTE_NONNULL can only be using to pointer.
> 
>>From 55cd85345b2dc50f44c1e382563482d40142382b Mon Sep 17 00:00:00 2001
> From: yubihong <yubihong@huawei.com>
> Date: Fri, 24 Apr 2020 17:44:43 +0800
> Subject: [PATCH] qemu: fix code format problem
> 
> Signed-off-by:Yu Bihong <yubihong@huawei.com>
> ---
>   src/libvirt-domain.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index a12809c..d659f1b 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -8194,11 +8194,11 @@ virDomainAttachDevice(virDomainPtr domain, const char *xml)
>       virCheckReadOnlyGoto(conn->flags, error);
> 
>       if (conn->driver->domainAttachDevice) {
> -       int ret;
> -       ret = conn->driver->domainAttachDevice(domain, xml);
> -       if (ret < 0)
> -          goto error;
> -       return ret;
> +        int ret;
> +        ret = conn->driver->domainAttachDevice(domain, xml);
> +        if (ret < 0)
> +            goto error;
> +        return ret;
>       }
> 
>       virReportUnsupportedError();
> @@ -8299,9 +8299,9 @@ virDomainDetachDevice(virDomainPtr domain, const char *xml)
>       if (conn->driver->domainDetachDevice) {
>           int ret;
>           ret = conn->driver->domainDetachDevice(domain, xml);
> -         if (ret < 0)
> -             goto error;
> -         return ret;
> +        if (ret < 0)
> +            goto error;
> +        return ret;
>        }
> 
>       virReportUnsupportedError();
> 

Re: bugifx: fix some wrong usage of ATTRIBUTE_NONNULL
Posted by Michal Privoznik 3 years, 10 months ago
On 6/8/20 7:52 PM, Daniel Henrique Barboza wrote:
> For the attached patch:
> 
> 
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

I'm not against the diff, but ...

> 
> On 6/6/20 7:30 AM, Bihong Yu wrote:
>> There are some wrong usage of ATTRIBUTE_NONNULL, which may cause the 
>> compilation fail. The
>> ATTRIBUTE_NONNULL is the define of __attribute__((__nonnull__(m))), 
>> which declares that the
>> input pointer parameter of funciton should not be NULL. If we declare 
>> some input pointer
>> parameter of the function is ATTRIBUTE_NONNULL, the function should 
>> not redundancy check of
>> the pointer parameter. And the ATTRIBUTE_NONNULL can only be using to 
>> pointer.
>>

This ^^ is unrelated (even though it's a mistake as there is another 
patch with this explanation that fixes ATTRIBUTE_NONNULL),

>>> From 55cd85345b2dc50f44c1e382563482d40142382b Mon Sep 17 00:00:00 2001
>> From: yubihong <yubihong@huawei.com>
>> Date: Fri, 24 Apr 2020 17:44:43 +0800
>> Subject: [PATCH] qemu: fix code format problem


But this is not qemu related. Can you please resend the patch with 
proper prefix? E.g. libvirt-domain: Fix indentation

And mention in the commit message what functions are fixed?

Thanks,
Michal

[libvirt PATCH v2] Fix some wrong usage of ATTRIBUTE_NONNULL
Posted by Bihong Yu 3 years, 10 months ago
There are some wrong usage of ATTRIBUTE_NONNULL, which may cause the compilation fail. The
ATTRIBUTE_NONNULL is the define of __attribute__((__nonnull__(m))), which declares that the
input pointer parameter of funciton should not be NULL. If we declare some input pointer
parameter of the function is ATTRIBUTE_NONNULL, the function should not redundancy check of
the pointer parameter. And the ATTRIBUTE_NONNULL can only be using to pointer.

>From 01a7301d50d6c1388df80dfa9af9da2582deec82 Mon Sep 17 00:00:00 2001
From: Bihong Yu <yubihong@huawei.com>
Date: Sat, 6 Jun 2020 18:20:16 +0800
Subject: [PATCH] bugifx: fix some wrong usage of ATTRIBUTE_NONNULL

The ATTRIBUTE_NONNULL is the define of __attribute__((__nonnull__(m))), which
declares that the input pointer parameter of funciton should not be NULL. If we
declare some input pointer parameter of the function is ATTRIBUTE_NONNULL, the
function should not redundancy check of the pointer parameter. And the
ATTRIBUTE_NONNULL can only be using to pointer.

Signed-off-by:Bihong Yu <yubihong@huawei.com>
Reviewed-by:Chuan Zheng <zhengchuan@huawei.com>
---
 src/libvirt_internal.h | 3 +--
 src/util/vircommand.h  | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
index 00ef7aa..72c6127 100644
--- a/src/libvirt_internal.h
+++ b/src/libvirt_internal.h
@@ -33,8 +33,7 @@ int virStateInitialize(bool privileged,
                        bool mandatory,
                        const char *root,
                        virStateInhibitCallback inhibit,
-                       void *opaque)
-    ATTRIBUTE_NONNULL(2);
+                       void *opaque);
 int virStateCleanup(void);
 int virStateReload(void);
 int virStateStop(void);
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index ff8a785..e12c88b 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -126,8 +126,7 @@ void virCommandAddArgFormat(virCommandPtr cmd,

 void virCommandAddArgPair(virCommandPtr cmd,
                           const char *name,
-                          const char *val)
-    ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+                          const char *val);

 void virCommandAddArgSet(virCommandPtr cmd,
                          const char *const*vals) ATTRIBUTE_NONNULL(2);
-- 
1.8.3.1

Re: [libvirt PATCH v2] Fix some wrong usage of ATTRIBUTE_NONNULL
Posted by Daniel Henrique Barboza 3 years, 10 months ago

On 6/6/20 7:52 AM, Bihong Yu wrote:
> There are some wrong usage of ATTRIBUTE_NONNULL, which may cause the compilation fail. The
> ATTRIBUTE_NONNULL is the define of __attribute__((__nonnull__(m))), which declares that the
> input pointer parameter of funciton should not be NULL. If we declare some input pointer
> parameter of the function is ATTRIBUTE_NONNULL, the function should not redundancy check of
> the pointer parameter. And the ATTRIBUTE_NONNULL can only be using to pointer.
> 
>>From 01a7301d50d6c1388df80dfa9af9da2582deec82 Mon Sep 17 00:00:00 2001
> From: Bihong Yu <yubihong@huawei.com>
> Date: Sat, 6 Jun 2020 18:20:16 +0800
> Subject: [PATCH] bugifx: fix some wrong usage of ATTRIBUTE_NONNULL
> 
> The ATTRIBUTE_NONNULL is the define of __attribute__((__nonnull__(m))), which
> declares that the input pointer parameter of funciton should not be NULL. If we
> declare some input pointer parameter of the function is ATTRIBUTE_NONNULL, the
> function should not redundancy check of the pointer parameter. And the
> ATTRIBUTE_NONNULL can only be using to pointer.
> 
> Signed-off-by:Bihong Yu <yubihong@huawei.com>
> Reviewed-by:Chuan Zheng <zhengchuan@huawei.com>
> ---


Patch looks ok (spoiler: didn't upload it to Gitlab to see if it passes CI).

But the commit msg needs a bit of work. The last paragraph is repeated in the
first paragraph, and I'm not sure about the email header in the middle of the
commit message.

Also, you mentioned "which may cause the compilation fail". It would be a good
plus to add which error message you were seeing that got fixed in this patch.


Thanks,


DHB


>   src/libvirt_internal.h | 3 +--
>   src/util/vircommand.h  | 3 +--
>   2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
> index 00ef7aa..72c6127 100644
> --- a/src/libvirt_internal.h
> +++ b/src/libvirt_internal.h
> @@ -33,8 +33,7 @@ int virStateInitialize(bool privileged,
>                          bool mandatory,
>                          const char *root,
>                          virStateInhibitCallback inhibit,
> -                       void *opaque)
> -    ATTRIBUTE_NONNULL(2);
> +                       void *opaque);
>   int virStateCleanup(void);
>   int virStateReload(void);
>   int virStateStop(void);
> diff --git a/src/util/vircommand.h b/src/util/vircommand.h
> index ff8a785..e12c88b 100644
> --- a/src/util/vircommand.h
> +++ b/src/util/vircommand.h
> @@ -126,8 +126,7 @@ void virCommandAddArgFormat(virCommandPtr cmd,
> 
>   void virCommandAddArgPair(virCommandPtr cmd,
>                             const char *name,
> -                          const char *val)
> -    ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
> +                          const char *val);
> 
>   void virCommandAddArgSet(virCommandPtr cmd,
>                            const char *const*vals) ATTRIBUTE_NONNULL(2);
> 

Re: [libvirt PATCH v2] Fix some wrong usage of ATTRIBUTE_NONNULL
Posted by Michal Privoznik 3 years, 10 months ago
On 6/6/20 12:52 PM, Bihong Yu wrote:
> There are some wrong usage of ATTRIBUTE_NONNULL, which may cause the compilation fail. The
> ATTRIBUTE_NONNULL is the define of __attribute__((__nonnull__(m))), which declares that the
> input pointer parameter of funciton should not be NULL. If we declare some input pointer
> parameter of the function is ATTRIBUTE_NONNULL, the function should not redundancy check of
> the pointer parameter. And the ATTRIBUTE_NONNULL can only be using to pointer.
> 
>>From 01a7301d50d6c1388df80dfa9af9da2582deec82 Mon Sep 17 00:00:00 2001
> From: Bihong Yu <yubihong@huawei.com>
> Date: Sat, 6 Jun 2020 18:20:16 +0800
> Subject: [PATCH] bugifx: fix some wrong usage of ATTRIBUTE_NONNULL
> 
> The ATTRIBUTE_NONNULL is the define of __attribute__((__nonnull__(m))), which
> declares that the input pointer parameter of funciton should not be NULL. If we
> declare some input pointer parameter of the function is ATTRIBUTE_NONNULL, the
> function should not redundancy check of the pointer parameter. And the
> ATTRIBUTE_NONNULL can only be using to pointer.
> 
> Signed-off-by:Bihong Yu <yubihong@huawei.com>
> Reviewed-by:Chuan Zheng <zhengchuan@huawei.com>
> ---
>   src/libvirt_internal.h | 3 +--
>   src/util/vircommand.h  | 3 +--
>   2 files changed, 2 insertions(+), 4 deletions(-)
> 

Tweaked the commit message a bit and pushed.

Congratulations on your first libvirt contribution!

Michal