> > This allows to move some low-level code out of reds.c > > Signed-off-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> > --- > server/net-utils.c | 25 +++++++++++++++++++++++++ > server/net-utils.h | 1 + > server/reds.c | 27 ++------------------------- > 3 files changed, 28 insertions(+), 25 deletions(-) > > diff --git a/server/net-utils.c b/server/net-utils.c > index ce409be..995b0d4 100644 > --- a/server/net-utils.c > +++ b/server/net-utils.c > @@ -31,6 +31,31 @@ > > #include "net-utils.h" > > +bool red_socket_set_keepalive(int fd, bool enable, int timeout) > +{ > + int keepalive = !!enable; > + > + if (setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &keepalive, > sizeof(keepalive)) == -1) { > + if (errno != ENOTSUP) { > + spice_printerr("setsockopt for keepalive failed, %s", > strerror(errno)); > + return false; > + } > + } > + > + if (!enable) { > + return true; > + } > + > + if (setsockopt(fd, SOL_TCP, TCP_KEEPIDLE, &timeout, sizeof(timeout)) == > -1) { About this value. NetBSD, Windows and Mac use milliseconds. If I would to write a portable wrapper I would go for milliseconds even if honestly I don't feel the need for such precision. > + if (errno != ENOTSUP) { > + spice_printerr("setsockopt for keepalive timeout failed, %s", > strerror(errno)); > + return false; > + } > + } > + > + return true; > +} > + > bool red_socket_set_no_delay(int fd, bool no_delay) > { > int delay_val = no_delay; > diff --git a/server/net-utils.h b/server/net-utils.h > index aed6956..d06932d 100644 > --- a/server/net-utils.h > +++ b/server/net-utils.h > @@ -18,6 +18,7 @@ > #ifndef _H_RED_NET_UTILS > #define _H_RED_NET_UTILS > > +bool red_socket_set_keepalive(int fd, bool enable, int timeout); Why not adding some documentation comment too? (this and other patches) > bool red_socket_set_no_delay(int fd, bool no_delay); > bool red_socket_set_non_blocking(int fd, bool non_blocking); > > diff --git a/server/reds.c b/server/reds.c > index 12c797b..024ff63 100644 > --- a/server/reds.c > +++ b/server/reds.c > @@ -73,6 +73,7 @@ > #include "main-channel-client.h" > #include "red-client.h" > #include "glib-compat.h" > +#include "net-utils.h" > > #define REDS_MAX_STAT_NODES 100 > > @@ -2377,39 +2378,15 @@ static void reds_handle_ssl_accept(int fd, int event, > void *data) > > #define KEEPALIVE_TIMEOUT (10*60) > > -static bool reds_init_keepalive(int socket) > -{ > - int keepalive = 1; > - int keepalive_timeout = KEEPALIVE_TIMEOUT; > - > - if (setsockopt(socket, SOL_SOCKET, SO_KEEPALIVE, &keepalive, > sizeof(keepalive)) == -1) { > - if (errno != ENOTSUP) { > - spice_printerr("setsockopt for keepalive failed, %s", > strerror(errno)); > - return false; > - } > - } > - > - if (setsockopt(socket, SOL_TCP, TCP_KEEPIDLE, > - &keepalive_timeout, sizeof(keepalive_timeout)) == -1) { > - if (errno != ENOTSUP) { > - spice_printerr("setsockopt for keepalive timeout failed, %s", > strerror(errno)); > - return false; > - } > - } > - > - return true; > -} > - > static RedLinkInfo *reds_init_client_connection(RedsState *reds, int socket) > { > RedLinkInfo *link; > > - reds_init_keepalive(socket); > - > if (!red_socket_set_non_blocking(socket, TRUE)) > { > goto error; > } > + red_socket_set_keepalive(socket, TRUE, KEEPALIVE_TIMEOUT); > Here, like in a previous order you revert the setting of these 2 flags (blocking and keep alive). If the end result is the same order I would always keep the same order. I assume you did some split of patches and didn't notice. > link = spice_new0(RedLinkInfo, 1); > link->reds = reds; Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel