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

[Markus Armbruster]

Max Reitz <address@hidden> 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 <address@hidden>
>> ---
>>  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 <address@hidden>

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;
>>  }
>>