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.
Improve a few error messages while there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
io/dns-resolver.c | 7 +++++--
ui/vnc.c | 18 ++++++++++++------
util/qemu-sockets.c | 4 +---
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/io/dns-resolver.c b/io/dns-resolver.c
index 0ac6b23..a407075 100644
--- a/io/dns-resolver.c
+++ b/io/dns-resolver.c
@@ -164,9 +164,12 @@ 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..0be77ba 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:
- error_setg(errp, "Unsupported socket kind %d",
- addr->type);
+ case SOCKET_ADDRESS_KIND_VSOCK:
+ case SOCKET_ADDRESS_KIND_FD:
+ error_setg(errp, "Unsupported socket address type %s",
+ SocketAddressKind_lookup[addr->type]);
break;
+ default:
+ abort();
}
return;
@@ -411,10 +414,13 @@ VncInfo *qmp_query_vnc(Error **errp)
info->family = NETWORK_ADDRESS_FAMILY_UNIX;
break;
- default:
- error_setg(errp, "Unsupported socket kind %d",
- addr->type);
+ case SOCKET_ADDRESS_KIND_VSOCK:
+ case SOCKET_ADDRESS_KIND_FD:
+ error_setg(errp, "Unsupported socket address type %s",
+ SocketAddressKind_lookup[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
On Thu, Mar 30, 2017 at 03:15:00PM +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. > > Improve a few error messages while there. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Max Reitz <mreitz@redhat.com> > --- > io/dns-resolver.c | 7 +++++-- > ui/vnc.c | 18 ++++++++++++------ > util/qemu-sockets.c | 4 +--- > 3 files changed, 18 insertions(+), 11 deletions(-) > > diff --git a/io/dns-resolver.c b/io/dns-resolver.c > index 0ac6b23..a407075 100644 > --- a/io/dns-resolver.c > +++ b/io/dns-resolver.c > @@ -164,9 +164,12 @@ 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(); > } > } Just repeating my v1 comments - this needs to be treating KIND_FD as a no-op, rather than raising 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/ :|
On 03.04.2017 13:48, Daniel P. Berrange wrote: > On Thu, Mar 30, 2017 at 03:15:00PM +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. >> >> Improve a few error messages while there. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> Reviewed-by: Max Reitz <mreitz@redhat.com> >> --- >> io/dns-resolver.c | 7 +++++-- >> ui/vnc.c | 18 ++++++++++++------ >> util/qemu-sockets.c | 4 +--- >> 3 files changed, 18 insertions(+), 11 deletions(-) >> >> diff --git a/io/dns-resolver.c b/io/dns-resolver.c >> index 0ac6b23..a407075 100644 >> --- a/io/dns-resolver.c >> +++ b/io/dns-resolver.c >> @@ -164,9 +164,12 @@ 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(); >> } >> } > > Just repeating my v1 comments - this needs to be treating KIND_FD as a no-op, > rather than raising an error. Do you want to write a patch? Markus is on vacation and since this is not a regression, dropping this patch from my queue wouldn't do any good either. Max
On Mon, Apr 03, 2017 at 02:50:12PM +0200, Max Reitz wrote: > On 03.04.2017 13:48, Daniel P. Berrange wrote: > > On Thu, Mar 30, 2017 at 03:15:00PM +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. > >> > >> Improve a few error messages while there. > >> > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> Reviewed-by: Max Reitz <mreitz@redhat.com> > >> --- > >> io/dns-resolver.c | 7 +++++-- > >> ui/vnc.c | 18 ++++++++++++------ > >> util/qemu-sockets.c | 4 +--- > >> 3 files changed, 18 insertions(+), 11 deletions(-) > >> > >> diff --git a/io/dns-resolver.c b/io/dns-resolver.c > >> index 0ac6b23..a407075 100644 > >> --- a/io/dns-resolver.c > >> +++ b/io/dns-resolver.c > >> @@ -164,9 +164,12 @@ 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(); > >> } > >> } > > > > Just repeating my v1 comments - this needs to be treating KIND_FD as a no-op, > > rather than raising an error. > > Do you want to write a patch? Markus is on vacation and since this is > not a regression, dropping this patch from my queue wouldn't do any good > either. Ok, i'll do a follow up. 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/ :|
On 03.04.2017 15:05, Daniel P. Berrange wrote: > On Mon, Apr 03, 2017 at 02:50:12PM +0200, Max Reitz wrote: >> On 03.04.2017 13:48, Daniel P. Berrange wrote: >>> On Thu, Mar 30, 2017 at 03:15:00PM +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. >>>> >>>> Improve a few error messages while there. >>>> >>>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>>> Reviewed-by: Max Reitz <mreitz@redhat.com> >>>> --- >>>> io/dns-resolver.c | 7 +++++-- >>>> ui/vnc.c | 18 ++++++++++++------ >>>> util/qemu-sockets.c | 4 +--- >>>> 3 files changed, 18 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/io/dns-resolver.c b/io/dns-resolver.c >>>> index 0ac6b23..a407075 100644 >>>> --- a/io/dns-resolver.c >>>> +++ b/io/dns-resolver.c >>>> @@ -164,9 +164,12 @@ 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(); >>>> } >>>> } >>> >>> Just repeating my v1 comments - this needs to be treating KIND_FD as a no-op, >>> rather than raising an error. >> >> Do you want to write a patch? Markus is on vacation and since this is >> not a regression, dropping this patch from my queue wouldn't do any good >> either. > > Ok, i'll do a follow up. Thanks a lot! Max
© 2016 - 2026 Red Hat, Inc.