[Qemu-devel] [for-2.9 3/8] io vnc sockets: Clean up SocketAddressKind switches

Markus Armbruster posted 8 patches 8 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [for-2.9 3/8] io vnc sockets: Clean up SocketAddressKind switches
Posted by Markus Armbruster 8 years, 10 months ago
We have quite a few switches over SocketAddressKind.  Some have case
labels for all enumeration values, others rely on a default label.
Some abort when the value isn't a valid SocketAddressKind, others
report an error then.

Unify as follows.  Always provide case labels for all enumeration
values, to clarify intent.  Abort when the value isn't a valid
SocketAddressKind, because the program state is messed up then.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 io/dns-resolver.c   |  6 ++++--
 ui/vnc.c            | 10 ++++++++--
 util/qemu-sockets.c |  4 +---
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/io/dns-resolver.c b/io/dns-resolver.c
index 0ac6b23..00fb575 100644
--- a/io/dns-resolver.c
+++ b/io/dns-resolver.c
@@ -164,9 +164,11 @@ int qio_dns_resolver_lookup_sync(QIODNSResolver *resolver,
                                                 addrs,
                                                 errp);
 
+    case SOCKET_ADDRESS_KIND_FD:
+        error_setg(errp, "Unsupported socket address type 'fd'");
+        return -1;
     default:
-        error_setg(errp, "Unknown socket address kind");
-        return -1;
+        abort();
     }
 }
 
diff --git a/ui/vnc.c b/ui/vnc.c
index fe0a46a..b6b58c4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -129,10 +129,13 @@ static void vnc_init_basic_info(SocketAddress *addr,
         info->family = NETWORK_ADDRESS_FAMILY_UNIX;
         break;
 
-    default:
+    case SOCKET_ADDRESS_KIND_VSOCK:
+    case SOCKET_ADDRESS_KIND_FD:
         error_setg(errp, "Unsupported socket kind %d",
                    addr->type);
         break;
+    default:
+        abort();
     }
 
     return;
@@ -411,10 +414,13 @@ VncInfo *qmp_query_vnc(Error **errp)
             info->family = NETWORK_ADDRESS_FAMILY_UNIX;
             break;
 
-        default:
+        case SOCKET_ADDRESS_KIND_VSOCK:
+        case SOCKET_ADDRESS_KIND_FD:
             error_setg(errp, "Unsupported socket kind %d",
                        addr->type);
             goto out_error;
+        default:
+            abort();
         }
 
         info->has_host = true;
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 9b73681..4ae37bd 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1337,9 +1337,7 @@ char *socket_address_to_string(struct SocketAddress *addr, Error **errp)
         break;
 
     default:
-        error_setg(errp, "socket family %d unsupported",
-                   addr->type);
-        return NULL;
+        abort();
     }
     return buf;
 }
-- 
2.7.4


Re: [Qemu-devel] [for-2.9 3/8] io vnc sockets: Clean up SocketAddressKind switches
Posted by Max Reitz 8 years, 10 months ago
On 29.03.2017 18:45, Markus Armbruster wrote:
> We have quite a few switches over SocketAddressKind.  Some have case
> labels for all enumeration values, others rely on a default label.
> Some abort when the value isn't a valid SocketAddressKind, others
> report an error then.
> 
> Unify as follows.  Always provide case labels for all enumeration
> values, to clarify intent.  Abort when the value isn't a valid
> SocketAddressKind, because the program state is messed up then.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  io/dns-resolver.c   |  6 ++++--
>  ui/vnc.c            | 10 ++++++++--
>  util/qemu-sockets.c |  4 +---
>  3 files changed, 13 insertions(+), 7 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Nit-picks below, as can be expected of me.

> diff --git a/io/dns-resolver.c b/io/dns-resolver.c
> index 0ac6b23..00fb575 100644
> --- a/io/dns-resolver.c
> +++ b/io/dns-resolver.c
> @@ -164,9 +164,11 @@ int qio_dns_resolver_lookup_sync(QIODNSResolver *resolver,
>                                                  addrs,
>                                                  errp);
>  
> +    case SOCKET_ADDRESS_KIND_FD:
> +        error_setg(errp, "Unsupported socket address type 'fd'");
> +        return -1;

Could do with an empty line here like the other cases have.

>      default:
> -        error_setg(errp, "Unknown socket address kind");
> -        return -1;
> +        abort();
>      }
>  }
>  
> diff --git a/ui/vnc.c b/ui/vnc.c
> index fe0a46a..b6b58c4 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -129,10 +129,13 @@ static void vnc_init_basic_info(SocketAddress *addr,
>          info->family = NETWORK_ADDRESS_FAMILY_UNIX;
>          break;
>  
> -    default:
> +    case SOCKET_ADDRESS_KIND_VSOCK:
> +    case SOCKET_ADDRESS_KIND_FD:
>          error_setg(errp, "Unsupported socket kind %d",
>                     addr->type);

Pre-existing, but: %s and SocketAddressKind_lookup[addr->type] would be
nicer.

>          break;
> +    default:
> +        abort();
>      }
>  
>      return;
> @@ -411,10 +414,13 @@ VncInfo *qmp_query_vnc(Error **errp)
>              info->family = NETWORK_ADDRESS_FAMILY_UNIX;
>              break;
>  
> -        default:
> +        case SOCKET_ADDRESS_KIND_VSOCK:
> +        case SOCKET_ADDRESS_KIND_FD:
>              error_setg(errp, "Unsupported socket kind %d",
>                         addr->type);

Same here.

Max

>              goto out_error;
> +        default:
> +            abort();
>          }
>  
>          info->has_host = true;
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 9b73681..4ae37bd 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1337,9 +1337,7 @@ char *socket_address_to_string(struct SocketAddress *addr, Error **errp)
>          break;
>  
>      default:
> -        error_setg(errp, "socket family %d unsupported",
> -                   addr->type);
> -        return NULL;
> +        abort();
>      }
>      return buf;
>  }
> 


Re: [Qemu-devel] [for-2.9 3/8] io vnc sockets: Clean up SocketAddressKind switches
Posted by Daniel P. Berrange 8 years, 10 months ago
On Wed, Mar 29, 2017 at 06:45:15PM +0200, Markus Armbruster wrote:
> We have quite a few switches over SocketAddressKind.  Some have case
> labels for all enumeration values, others rely on a default label.
> Some abort when the value isn't a valid SocketAddressKind, others
> report an error then.
> 
> Unify as follows.  Always provide case labels for all enumeration
> values, to clarify intent.  Abort when the value isn't a valid
> SocketAddressKind, because the program state is messed up then.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  io/dns-resolver.c   |  6 ++++--
>  ui/vnc.c            | 10 ++++++++--
>  util/qemu-sockets.c |  4 +---
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/io/dns-resolver.c b/io/dns-resolver.c
> index 0ac6b23..00fb575 100644
> --- a/io/dns-resolver.c
> +++ b/io/dns-resolver.c
> @@ -164,9 +164,11 @@ int qio_dns_resolver_lookup_sync(QIODNSResolver *resolver,
>                                                  addrs,
>                                                  errp);
>  
> +    case SOCKET_ADDRESS_KIND_FD:
> +        error_setg(errp, "Unsupported socket address type 'fd'");
> +        return -1;
>      default:
> -        error_setg(errp, "Unknown socket address kind");
> -        return -1;
> +        abort();
>      }
>  }

KIND_FD needs to be handled the same way as KIND_UNIX/VSOCK - ie a no-op,
not an error.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|