| Message ID | 20171221155905.3793-2-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] io: move fd_is_socket() into common sockets code | expand |
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*.
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); } With or without such tweaks: Reviewed-by: Markus Armbruster <armbru@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
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index 4f7311b52a..5680880f5a 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -12,6 +12,7 @@ int inet_aton(const char *cp, struct in_addr *ia); #include "qapi-types.h" /* misc helpers */ +bool fd_is_socket(int fd); int qemu_socket(int domain, int type, int protocol); int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen); int socket_set_cork(int fd, int v); diff --git a/io/channel-util.c b/io/channel-util.c index 0fb4bd0837..423d79845a 100644 --- a/io/channel-util.c +++ b/io/channel-util.c @@ -24,19 +24,6 @@ #include "io/channel-socket.h" -static bool fd_is_socket(int fd) -{ - int optval; - socklen_t optlen; - optlen = sizeof(optval); - return qemu_getsockopt(fd, - SOL_SOCKET, - SO_TYPE, - (char *)&optval, - &optlen) == 0; -} - - QIOChannel *qio_channel_new_fd(int fd, Error **errp) { diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index af4f01211a..1d23f0b742 100644 --- a/util/qemu-sockets.c +++ 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, + &optlen) == 0; +} + + /* * Matrix we're trying to apply *
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(-)