Dpi_connect_socket retry

[email protected]
Newsgroups gmane.comp.web.dillo.devel
Message-ID <20121209105843.GA11938@darkstar> 
Dpi_connect_socket function from src/IO/dpi.c (also duplicated in
test/cookies.c) has retry argument. It is set to TRUE on first call,
and Dpi_connect_socket calls itself recursively with retry == FALSE in
case of error.

This retry function is not working. If connection fails, at line 645
sock_fd is set, but ret is still set to -1. Even if the second attempt
to connect is successful, -1 is returned and conected sock_fd is lost.

It is possible to fix it by adding "ret = sock_fd", but I think it is
better to just remove retries completely. It is unlikely that
connecting using loopback interface will fail on first try and succeed
on retry.

Also, comment says "the server closes sock_fd on error". However,
sock_fd should still be closed by client to free allocated file
descriptor. OS don't know if Dillo is going to read from it (in which
case error such as ECONNRESET should happen) so file descriptor can't
be reused.

File descriptor should be closed in case of error in a_Dpip_build_cmd
too.

Patch to fix this function and its calls in src/IO/dpi.c and
test/cookies.c is attached.

_______________________________________________
Dillo-dev mailing list
[email protected]
http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev
Dpi_connect_socket.patch (text/plain, 4.7 KB) 
diff -r 6a525e279c5d src/IO/dpi.c
--- a/src/IO/dpi.c
+++ b/src/IO/dpi.c
@@ -614,10 +614,10 @@
  * We have to ask 'dpid' (dpi daemon) for the port of the target dpi server.
  * Once we have it, then the proper file descriptor is returned (-1 on error).
  */
-static int Dpi_connect_socket(const char *server_name, int retry)
+static int Dpi_connect_socket(const char *server_name)
 {
    struct sockaddr_in sin;
-   int sock_fd, err, dpi_port, ret=-1;
+   int sock_fd, dpi_port, ret = -1;
    char *cmd = NULL;
 
    /* Query dpid for the port number for this server */
@@ -633,28 +633,18 @@
    sin.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
    sin.sin_port = htons(dpi_port);
 
-   if ((sock_fd = Dpi_make_socket_fd()) == -1) {
+   if ((sock_fd = Dpi_make_socket_fd()) == -1)
       perror("[dpi::socket]");
-   } else if (connect(sock_fd, (void*)&sin, sizeof(sin)) == -1) {
-      err = errno;
-      sock_fd = -1;
+   else if (connect(sock_fd, (void*)&sin, sizeof(sin)) == -1)
       MSG("[dpi::connect] errno:%d %s\n", errno, dStrerror(errno));
-      if (retry) {
-         switch (err) {
-            case ECONNREFUSED: case EBADF: case ENOTSOCK: case EADDRNOTAVAIL:
-               sock_fd = Dpi_connect_socket(server_name, FALSE);
-               break;
-         }
-      }
-
-   /* send authentication Key (the server closes sock_fd on error) */
-   } else if (!(cmd = a_Dpip_build_cmd("cmd=%s msg=%s", "auth", SharedKey))) {
+   else if (!(cmd = a_Dpip_build_cmd("cmd=%s msg=%s", "auth", SharedKey)))
       MSG_ERR("[Dpi_connect_socket] Can't make auth message.\n");
-   } else if (Dpi_blocking_write(sock_fd, cmd, strlen(cmd)) == -1) {
+   else if (Dpi_blocking_write(sock_fd, cmd, strlen(cmd)) == -1)
       MSG_ERR("[Dpi_connect_socket] Can't send auth message.\n");
-   } else {
+   else
       ret = sock_fd;
-   }
+   if (ret == -1)
+      Dpi_close_fd(sock_fd); 
    dFree(cmd);
 
    return ret;
@@ -677,7 +667,7 @@
          switch (Op) {
          case OpStart:
             if ((st = Dpi_blocking_start_dpid()) == 0) {
-               SockFD = Dpi_connect_socket(Data1, TRUE);
+               SockFD = Dpi_connect_socket(Data1);
                if (SockFD != -1) {
                   int *fd = dNew(int, 1);
                   *fd = SockFD;
@@ -798,7 +788,7 @@
       return ret;
    }
 
-   if ((sock_fd = Dpi_connect_socket(server_name, TRUE)) == -1) {
+   if ((sock_fd = Dpi_connect_socket(server_name)) == -1) {
       MSG_ERR("[a_Dpi_send_blocking_cmd] Can't connect to server.\n");
    } else if (Dpi_blocking_write(sock_fd, cmd, strlen(cmd)) == -1) {
       MSG_ERR("[a_Dpi_send_blocking_cmd] Can't send message.\n");
diff -r 6a525e279c5d test/cookies.c
--- a/test/cookies.c
+++ b/test/cookies.c
@@ -368,10 +368,11 @@
 }
 
 
-static int Dpi_connect_socket(const char *server_name, int retry)
+
+static int Dpi_connect_socket(const char *server_name)
 {
    struct sockaddr_in sin;
-   int sock_fd, err, dpi_port, ret=-1;
+   int sock_fd, dpi_port, ret = -1;
    char *cmd = NULL;
 
    /* Query dpid for the port number for this server */
@@ -387,28 +388,18 @@
    sin.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
    sin.sin_port = htons(dpi_port);
 
-   if ((sock_fd = Dpi_make_socket_fd()) == -1) {
+   if ((sock_fd = Dpi_make_socket_fd()) == -1)
       perror("[dpi::socket]");
-   } else if (connect(sock_fd, (void*)&sin, sizeof(sin)) == -1) {
-      err = errno;
-      sock_fd = -1;
+   else if (connect(sock_fd, (void*)&sin, sizeof(sin)) == -1)
       MSG("[dpi::connect] errno:%d %s\n", errno, dStrerror(errno));
-      if (retry) {
-         switch (err) {
-            case ECONNREFUSED: case EBADF: case ENOTSOCK: case EADDRNOTAVAIL:
-               sock_fd = Dpi_connect_socket(server_name, FALSE);
-               break;
-         }
-      }
-
-   /* send authentication Key (the server closes sock_fd on error) */
-   } else if (!(cmd = a_Dpip_build_cmd("cmd=%s msg=%s", "auth", SharedKey))) {
+   else if (!(cmd = a_Dpip_build_cmd("cmd=%s msg=%s", "auth", SharedKey)))
       MSG_ERR("[Dpi_connect_socket] Can't make auth message.\n");
-   } else if (Dpi_blocking_write(sock_fd, cmd, strlen(cmd)) == -1) {
+   else if (Dpi_blocking_write(sock_fd, cmd, strlen(cmd)) == -1)
       MSG_ERR("[Dpi_connect_socket] Can't send auth message.\n");
-   } else {
+   else
       ret = sock_fd;
-   }
+   if (ret == -1)
+      Dpi_close_fd(sock_fd); 
    dFree(cmd);
 
    return ret;
@@ -425,7 +416,7 @@
       return ret;
    }
 
-   if ((sock_fd = Dpi_connect_socket(server_name, TRUE)) == -1) {
+   if ((sock_fd = Dpi_connect_socket(server_name)) == -1) {
       MSG_ERR("[a_Dpi_send_blocking_cmd] Can't connect to server.\n");
    } else if (Dpi_blocking_write(sock_fd, cmd, strlen(cmd)) == -1) {
       MSG_ERR("[a_Dpi_send_blocking_cmd] Can't send message.\n");
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.