Yonghong Song <yhs@xxxxxx> [Thu, 2020-05-14 08:56 -0700]:
> On 5/13/20 2:38 PM, Andrey Ignatov wrote:
> > @@ -77,8 +81,6 @@ static const size_t timeo_optlen = sizeof(timeo_sec);
> > int connect_to_fd(int family, int type, int server_fd)
> > {
> > - struct sockaddr_storage addr;
> > - socklen_t len = sizeof(addr);
> > int fd;
> > fd = socket(family, type, 0);
> > @@ -87,24 +89,64 @@ int connect_to_fd(int family, int type, int server_fd)
> > return -1;
> > }
> > - if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeo_sec, timeo_optlen)) {
> > + if (connect_fd_to_fd(fd, server_fd) < 0 && errno != EINPROGRESS) {
> > + close(fd);
>
> Remote possibility. close(fd) may change error code?
It can in some cases that are rather theoritical in this case (e.g.
buggy multi-threaded program closes fd from another thread right before
this close()).
But I can save/restore it before/after close just in case.
> In my opinion, maybe convert the original syscall failure errno to return
> value and carrying on might be a simpler choice?
I wanted to preserve semantics of connect(2) here: return -1 on error,
or fd >= 0 on success.
I guess if I save/restore errno it should be fine.
> > + return -1;
> > + }
> > +
> > + return fd;
> > +}
> > +
> > +int connect_fd_to_fd(int client_fd, int server_fd)
> > +{
> > + struct sockaddr_storage addr;
> > + socklen_t len = sizeof(addr);
> > +
> > + if (setsockopt(client_fd, SOL_SOCKET, SO_RCVTIMEO, &timeo_sec,
> > + timeo_optlen)) {
> > log_err("Failed to set SO_RCVTIMEO");
> > - goto out;
> > + return -1;
> > }
> > if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
> > log_err("Failed to get server addr");
> > - goto out;
> > + return -1;
> > }
> > - if (connect(fd, (const struct sockaddr *)&addr, len) < 0) {
> > - log_err("Fail to connect to server with family %d", family);
> > - goto out;
> > + if (connect(client_fd, (const struct sockaddr *)&addr, len) < 0) {
> > + if (errno != EINPROGRESS)
> > + log_err("Failed to connect to server");
>
> Not saying it is possible, but any remote possibility log_err()
> may change error code to EINPROGRESS?
To my best knowledge, neither fprintf(3) nor strerror(3) use
EINPROGRESS, but since in this case having a reliable way to communicate
EINPROGRESS from connect is rather required, I'll save/restore errno for
caling log_err.
--
Andrey Ignatov
