Commit id 'adf846d3' added many ATTRIBUTE_FALLTHROUGH; statements;
however, this particular one caught the eye of the Coverity checker
which notes that the statement isn't reachable when the # if is true.

While it could be considered a false positive, just add an # else in
order to make it more obvious.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/util/virnetdevbridge.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index 11b03b4..1361a51 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -478,11 +478,12 @@ virNetDevBridgeCreate(const char *brname)
              */
             rc = virNetDevBridgeCreateWithIoctl(brname);
             goto cleanup;
-# endif
+# else
             /* intentionally fall through if virNetDevBridgeCreateWithIoctl()
              * isn't available.
              */
             ATTRIBUTE_FALLTHROUGH;
+# endif
         default:
             virReportSystemError(-err->error,
                                  _("error creating bridge interface %s"),
-- 
2.9.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, Jun 28, 2017 at 07:37:37 -0400, John Ferlan wrote:
> Commit id 'adf846d3' added many ATTRIBUTE_FALLTHROUGH; statements;
> however, this particular one caught the eye of the Coverity checker
> which notes that the statement isn't reachable when the # if is true.
> 
> While it could be considered a false positive, just add an # else in
> order to make it more obvious.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---

I think this is a cleaner solution, since it gets rid of that weird
swtich statement altogether:

diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index 11b03b426..cfb7ebae9 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -468,22 +468,17 @@ virNetDevBridgeCreate(const char *brname)
         if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
             goto malformed_resp;

-        switch (err->error) {
-        case 0:
-            break;
-        case -EOPNOTSUPP:
+        if (err->error < 0) {
 # if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR)
-            /* fallback to ioctl if netlink doesn't support creating
-             * bridges
-             */
-            rc = virNetDevBridgeCreateWithIoctl(brname);
-            goto cleanup;
+            if (err->error == -EOPNOTSUPP) {
+                /* fallback to ioctl if netlink doesn't support creating
+                 * bridges
+                 */
+                rc = virNetDevBridgeCreateWithIoctl(brname);
+                goto cleanup;
+            }
 # endif
-            /* intentionally fall through if virNetDevBridgeCreateWithIoctl()
-             * isn't available.
-             */
-            ATTRIBUTE_FALLTHROUGH;
-        default:
+
             virReportSystemError(-err->error,
                                  _("error creating bridge interface %s"),
                                  brname);


I'll send it as a formal patch if you agree.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 06/28/2017 08:01 AM, Peter Krempa wrote:
> On Wed, Jun 28, 2017 at 07:37:37 -0400, John Ferlan wrote:
>> Commit id 'adf846d3' added many ATTRIBUTE_FALLTHROUGH; statements;
>> however, this particular one caught the eye of the Coverity checker
>> which notes that the statement isn't reachable when the # if is true.
>>
>> While it could be considered a false positive, just add an # else in
>> order to make it more obvious.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
> 
> I think this is a cleaner solution, since it gets rid of that weird
> swtich statement altogether:
> 

That's fine.  I'll drop this one.

John

> diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
> index 11b03b426..cfb7ebae9 100644
> --- a/src/util/virnetdevbridge.c
> +++ b/src/util/virnetdevbridge.c
> @@ -468,22 +468,17 @@ virNetDevBridgeCreate(const char *brname)
>          if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
>              goto malformed_resp;
> 
> -        switch (err->error) {
> -        case 0:
> -            break;
> -        case -EOPNOTSUPP:
> +        if (err->error < 0) {
>  # if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR)
> -            /* fallback to ioctl if netlink doesn't support creating
> -             * bridges
> -             */
> -            rc = virNetDevBridgeCreateWithIoctl(brname);
> -            goto cleanup;
> +            if (err->error == -EOPNOTSUPP) {
> +                /* fallback to ioctl if netlink doesn't support creating
> +                 * bridges
> +                 */
> +                rc = virNetDevBridgeCreateWithIoctl(brname);
> +                goto cleanup;
> +            }
>  # endif
> -            /* intentionally fall through if virNetDevBridgeCreateWithIoctl()
> -             * isn't available.
> -             */
> -            ATTRIBUTE_FALLTHROUGH;
> -        default:
> +
>              virReportSystemError(-err->error,
>                                   _("error creating bridge interface %s"),
>                                   brname);
> 
> 
> I'll send it as a formal patch if you agree.
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list