On Tue, Aug 02, 2022 at 04:15:39PM +0200, Claudio Jeker wrote:
> On Tue, Aug 02, 2022 at 12:42:26PM +0000, Job Snijders wrote:
> > This adds '--contimeout' to rsync(1)
> >
> > $ time openrsync --contimeout=5 -rt rsync://203.119.21.1/test /tmp/k
> > openrsync: warning: connect timeout: 203.119.21.1, 203.119.21.1
> > openrsync: error: cannot connect to host: 203.119.21.1
> > 0m05.01s real 0m00.00s user 0m00.01s system
> >
> > OK?
> >
...
> > @@ -99,8 +103,28 @@ inet_connect(int *sd, const struct sourc
>
> You make the socket non-blocking by default. Please remove the fcntl
> at the end of the function. There is no need to double up the
> non-blocking here.
>
> Also I would move the ETIMEDOUT handling up into the connect block. No
> need to do this complicated errno game.
>
> You can use a switch for the poll call: switch (poll()) { case 0, case
> 1, default). I don't like how c is used over and over again.
Thanks for the feedback. Like so?
Kind regards,
Job
Index: socket.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/socket.c,v
retrieving revision 1.31
diff -u -p -r1.31 socket.c
--- socket.c 30 Jun 2021 13:10:04 -0000 1.31
+++ socket.c 2 Aug 2022 16:54:45 -0000
@@ -75,14 +75,18 @@ static int
inet_connect(int *sd, const struct source *src, const char *host,
const struct source *bsrc, size_t bsrcsz)
{
- int c, flags;
+ struct pollfd pfd;
+ socklen_t optlen;
+ int c, flags;
+ int optval;
if (*sd != -1)
close(*sd);
LOG2("trying: %s, %s", src->ip, host);
- if ((*sd = socket(src->family, SOCK_STREAM, 0)) == -1) {
+ if ((*sd = socket(src->family, SOCK_STREAM | SOCK_NONBLOCK, 0))
+ == -1) {
ERR("socket");
return -1;
}
@@ -94,30 +98,42 @@ inet_connect(int *sd, const struct sourc
/*
* Initiate blocking connection.
- * We use the blocking connect() instead of passing NONBLOCK to
- * the socket() function because we don't need to do anything
- * while waiting for this to finish.
+ * We use non-blocking connect() so we can poll() for contimeout.
*/
- c = connect(*sd, (const struct sockaddr *)&src->sa, src->salen);
+ if ((c = connect(*sd, (const struct sockaddr *)&src->sa, src->salen))
+ != 0 && errno == EINPROGRESS) {
+ pfd.fd = *sd;
+ pfd.events = POLLOUT;
+ switch (c = poll(&pfd, 1, poll_contimeout)) {
+ case 1:
+ optlen = sizeof(optval);
+ if ((c = getsockopt(*sd, SOL_SOCKET, SO_ERROR, &optval,
+ &optlen)) == 0) {
+ errno = optval;
+ c = optval == 0 ? 0 : -1;
+ }
+ break;
+ case 0:
+ errno = ETIMEDOUT;
+ WARNX("connect timeout: %s, %s", src->ip, host);
+ return 0;
+ default:
+ err(1, "poll failed");
+ }
+ }
if (c == -1) {
if (errno == EADDRNOTAVAIL)
return 0;
if (errno == ECONNREFUSED || errno == EHOSTUNREACH) {
- WARNX("connect refused: %s, %s",
- src->ip, host);
+ WARNX("connect refused: %s, %s", src->ip, host);
return 0;
}
ERR("connect");
return -1;
}
- /* Set up non-blocking mode. */
-
if ((flags = fcntl(*sd, F_GETFL, 0)) == -1) {
- ERR("fcntl");
- return -1;
- } else if (fcntl(*sd, F_SETFL, flags|O_NONBLOCK) == -1) {
ERR("fcntl");
return -1;
}
