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

["Daniel P. Berrange" <berrange@redhat.com>]

On Fri, Dec 22, 2017 at 09:55:29AM +0100, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> 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 <berrange@redhat.com>
> >> ---
> >>   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 <eblake@redhat.com>
> >
> > 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);
>     }

Yes I'll do exactly that.

> 
> With or without such tweaks:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|