Max Reitz <mre...@redhat.com> writes: > 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 <arm...@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 <mre...@redhat.com>
Thanks! > Nit-picks below, as can be expected of me. Bring 'em on! >> 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. Indeed. >> 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. Can do. >> 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. Yup. > 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; >> } >>
