Re: [Qemu-devel] [PATCH v2 1/2] io: move fd_is_socket() into common sockets code

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 12/21/2017 09:59 AM, Daniel P. Berrange wrote:
>> The fd_is_socket() helper method is useful in a few places, so put it in
>> the common sockets code.
>>
>> Signed-off-by: Daniel P. Berrange <address@hidden>
>> ---
>>   include/qemu/sockets.h |  1 +
>>   io/channel-util.c      | 13 -------------
>>   util/qemu-sockets.c    | 13 +++++++++++++
>>   3 files changed, 14 insertions(+), 13 deletions(-)
>
> Reviewed-by: Eric Blake <address@hidden>
>
> But while you are touching this...
>
>> +++ b/util/qemu-sockets.c
>> @@ -91,6 +91,19 @@ NetworkAddressFamily inet_netfamily(int family)
>>       return NETWORK_ADDRESS_FAMILY_UNKNOWN;
>>   }
>>   +bool fd_is_socket(int fd)
>> +{
>> +    int optval;
>> +    socklen_t optlen;
>> +    optlen = sizeof(optval);
>> +    return qemu_getsockopt(fd,
>> +                           SOL_SOCKET,
>> +                           SO_TYPE,
>> +                           (char *)&optval,
>
> This cast is pointless (although you are just moving it from the old
> code).  qemu_getsockopt() already takes care of casting for mingw
> (where the signature is not POSIX-compliant), and on all other
> platforms, the argument is already prototyped as void*; and since
> void* accepts anything, you don't have to go through an intermediate
> char*.

If we're tweaking, then I'd like it tweaked so:

    bool fd_is_socket(int fd)
    {
        int optval;
        socklen_t optlen = sizeof(optval);

        return !qemu_getsockopt(fd, SOL_SOCKET, SO_TYPE, &optval, &optlen);
    }

With or without such tweaks:
Reviewed-by: Markus Armbruster <address@hidden>