changeset 1389:c1c822f70582

Removed the write/fwrite mix in dpip. Introduced a_Dpip_dsh_trywrite() Several cleanups and more error handling sprinkled all over too.
author Jorge Arellano Cid <jcid@dillo.org>
date Sun, 01 Nov 2009 16:31:59 -0300
parents eb35203124e4
children 27911457287d
files dpi/file.c dpid/dpid.c dpid/dpid_common.c dpid/dpid_common.h dpip/dpip.c dpip/dpip.h src/IO/dpi.c src/auth.c
diffstat 8 files changed, 195 insertions(+), 56 deletions(-) [+]
line wrap: on
line diff
--- a/dpi/file.c	Sun Nov 01 16:31:59 2009 -0300
+++ b/dpi/file.c	Sun Nov 01 16:31:59 2009 -0300
@@ -60,7 +60,7 @@
 
 
 typedef enum {
-   st_start,
+   st_start = 10,
    st_dpip,
    st_http,
    st_content,
@@ -92,6 +92,8 @@
    int err_code;
    int flags;
    int old_style;
+
+   Dstr *dbuf;
 } ClientInfo;
 
 /*
@@ -115,7 +117,7 @@
  */
 static void File_close(int fd)
 {
-   while (close(fd) < 0 && errno == EINTR)
+   while (fd >= 0 && close(fd) < 0 && errno == EINTR)
       ;
 }
 
@@ -547,7 +549,7 @@
          File_close(fd);
       }
    }
-
+   _MSG("File_content_type: name=%s ct=%s\n", filename, ct);
    return ct;
 }
 
@@ -697,7 +699,7 @@
    const char *ct;
    const char *unknown_type = "application/octet-stream";
    char buf[LBUF], *d_cmd, *name;
-   int st, namelen;
+   int st, st2, namelen;
    bool_t gzipped = FALSE;
 
    if (client->state == st_start) {
@@ -744,27 +746,30 @@
 
    } else if (client->state == st_http) {
       /* Send body -- raw file contents */
-      do {
-         if ((st = read(client->file_fd, buf, LBUF)) > 0) {
-            if (a_Dpip_dsh_write(client->sh, 0, buf, (size_t)st) != 0)
-               break;
-         } else if (st < 0) {
-            perror("[read]");
-            if (errno == EINTR || errno == EAGAIN)
-               continue;
+      if (client->dbuf->len > 0) {
+         /* send pending data */
+         st = a_Dpip_dsh_trywrite(client->sh, 
+                                  client->dbuf->str, client->dbuf->len);
+         if (st > 0)
+            dStr_erase(client->dbuf, 0, st);
+      } else {
+         /* ok to send new data */
+         do {
+            st = read(client->file_fd, buf, LBUF);
+         } while (st < 0 && errno == EINTR);
+         if (st == -1) {
+            MSG("\nexit(1), ERROR while reading from file '%s': %s\n\n",
+                client->filename, dStrerror(errno));
+            exit(1);
+         } else if (st == 0) {
+            client->state = st_content;
+            client->flags |= FILE_DONE;
+         } else {
+            /* partial write */
+            st2 = a_Dpip_dsh_trywrite(client->sh, buf, st);
+            if (st2 > 0 && st2 < st)
+               dStr_append_l(client->dbuf, buf + st2, st - st2);
          }
-      } while (st > 0);
-   
-      /* TODO: It may be better to send an error report to dillo instead of
-       *       calling exit() */
-      if (st == -1) {
-         MSG("ERROR while reading from file \"%s\", error was \"%s\"\n",
-             client->filename, dStrerror(errno));
-         exit(1);
-      } else if (st == 0) {
-         client->state = st_content;
-         File_close(client->file_fd);
-         client->flags |= FILE_DONE;
       }
    }
 
@@ -859,6 +864,7 @@
  */
 static void termination_handler(int signum)
 {
+  MSG("\nexit(signum), signum=%d\n\n", signum);
   exit(signum);
 }
 
@@ -883,6 +889,8 @@
    new_client->err_code = 0;
    new_client->flags = FILE_READ;
    new_client->old_style = OLD_STYLE;
+   new_client->dbuf = dStr_sized_new(8*1024);
+
    dList_append(Clients, new_client);
    return new_client;
 }
@@ -897,8 +905,10 @@
    _MSG("Closing Socket Handler\n");
    a_Dpip_dsh_close(client->sh);
    a_Dpip_dsh_free(client->sh);
+   File_close(client->file_fd);
    dFree(client->orig_url);
    dFree(client->filename);
+   dStr_free(client->dbuf, TRUE);
    File_dillodir_free(client->d_dir);
 
    dFree(client);
@@ -914,7 +924,8 @@
    int st;
 
    while (1) {
-      MSG("File_serve_client %p, flags=%d\n", client, client->flags);
+      _MSG("File_serve_client %p, flags=%d state=%d\n",
+          client, client->flags, client->state);
       if (client->flags & (FILE_DONE | FILE_ERR))
          break;
       if (client->flags & FILE_READ) {
@@ -1015,7 +1026,7 @@
       if (client->flags & FILE_WRITE)
          FD_SET (client->sh->fd_out, &write_set);
    }
-   MSG("Watching %d fds:", dList_length(Clients) + 1);
+   MSG("Watching %d fds\n", dList_length(Clients) + 1);
 
    /* Initialize the timeout data structure. */
    timeout.tv_sec = seconds;
@@ -1024,7 +1035,7 @@
    do {
       st = select(FD_SETSIZE, &read_set, &write_set, NULL, &timeout);
    } while (st == -1 && errno == EINTR);
-
+/*
    MSG_RAW(" (%d%s%s)", STDIN_FILENO,
            FD_ISSET(STDIN_FILENO, &read_set) ? "R" : "",
            FD_ISSET(STDIN_FILENO, &write_set) ? "W" : "");
@@ -1034,7 +1045,7 @@
               FD_ISSET(client->sh->fd_out, &write_set) ? "W" : "");
    }
    MSG_RAW("\n");
- 
+*/
    return st;
 }
 
@@ -1087,6 +1098,7 @@
             break;
          } else {
             /* Create and initialize a new client */
+            MSG(" accept() fd=%d\n", tmp_fd);
             File_add_client(tmp_fd);
          }
          continue;
--- a/dpid/dpid.c	Sun Nov 01 16:31:59 2009 -0300
+++ b/dpid/dpid.c	Sun Nov 01 16:31:59 2009 -0300
@@ -513,6 +513,25 @@
    return (dList_length(*services_list));
 }
 
+/*
+ * Return a socket file descriptor
+ * (useful to set socket options in a uniform way)
+ */
+static int make_socket_fd()
+{
+   int ret;
+
+   if ((ret = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
+      ERRMSG("make_socket_fd", "socket", errno);
+   }
+
+   /* set some buffering to increase the transfer's speed */
+   //setsockopt(sock_fd, SOL_SOCKET, SO_SNDBUF,
+   //           &sock_buflen, (socklen_t)sizeof(sock_buflen));
+
+   return ret;
+}
+
 /*! Bind a socket port on localhost. Try to be close to base_port.
  * \Return
  * \li listening socket file descriptor on success
@@ -522,19 +541,14 @@
 {
    int sock_fd, port;
    struct sockaddr_in sin;
-   //size_t sock_buflen = 8192;
    int ok = 0, last_port = base_port + 50;
 
-   if ((sock_fd = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
-      ERRMSG("bind_socket_fd", "socket", errno);
+   if ((sock_fd = make_socket_fd()) == -1) {
       return (-1);              /* avoids nested ifs */
    }
    /* Set the socket FD to close on exec */
    fcntl(sock_fd, F_SETFD, FD_CLOEXEC | fcntl(sock_fd, F_GETFD));
 
-   /* set some buffering to increase the transfer's speed */
-   //setsockopt(sock_fd, SOL_SOCKET, SO_SNDBUF,
-   //           &sock_buflen, (socklen_t)sizeof(sock_buflen));
 
    memset(&sin, 0, sizeof(sin));
    sin.sin_family = AF_INET;
@@ -579,8 +593,9 @@
       MSG("save_comm_keys: open %s\n", dStrerror(errno));
    } else {
       snprintf(port_str, 16, "%d %s\n", srs_port, SharedKey);
-      if (CKD_WRITE(fd, port_str) != -1)
+      if (CKD_WRITE(fd, port_str) != -1 && CKD_CLOSE(fd) != -1) {
          ret = 1;
+      }
    }
 
    return ret;
@@ -734,10 +749,11 @@
       if (dpi_attr_list[i].pid == 1 || dpi_attr_list[i].filter)
          continue;
 
-      if ((sock_fd = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
+      if ((sock_fd = make_socket_fd()) == -1) {
          ERRMSG("stop_active_dpis", "socket", errno);
          continue;
       }
+
       sin.sin_port = htons(dpi_attr_list[i].port);
       if (ckd_connect(sock_fd, (struct sockaddr *)&sin, sizeof(sin)) == -1) {
          ERRMSG("stop_active_dpis", "connect", errno);
--- a/dpid/dpid_common.c	Sun Nov 01 16:31:59 2009 -0300
+++ b/dpid/dpid_common.c	Sun Nov 01 16:31:59 2009 -0300
@@ -52,3 +52,22 @@
    }
    return (ret);
 }
+
+/*!
+ * Provides an error checked close() call.
+ * Call this via the CKD_CLOSE macro
+ * \return close return value
+ */
+ssize_t ckd_close(int fd, char *file, int line)
+{
+   ssize_t ret;
+
+   do {
+      ret = close(fd);
+   } while (ret == -1 && errno == EINTR);
+   if (ret == -1) {
+      MSG_ERR("%s:%d: close: %s\n", file, line, dStrerror(errno));
+   }
+   return (ret);
+}
+
--- a/dpid/dpid_common.h	Sun Nov 01 16:31:59 2009 -0300
+++ b/dpid/dpid_common.h	Sun Nov 01 16:31:59 2009 -0300
@@ -31,9 +31,10 @@
 
 
 /*!
- * Macro for calling the ckd_write function
+ * Macros for calling ckd_write and ckd_close functions
  */
 #define CKD_WRITE(fd, msg) ckd_write(fd, msg, __FILE__, __LINE__)
+#define CKD_CLOSE(fd)      ckd_close(fd, __FILE__, __LINE__)
 
 
 /*! Error codes for dpid */
@@ -56,5 +57,6 @@
 int no_dotfiles(const struct dirent *filedat);
 
 ssize_t ckd_write(int fd, char *msg, char *file, int line);
+ssize_t ckd_close(int fd, char *file, int line);
 
 #endif
--- a/dpip/dpip.c	Sun Nov 01 16:31:59 2009 -0300
+++ b/dpip/dpip.c	Sun Nov 01 16:31:59 2009 -0300
@@ -221,6 +221,8 @@
       if (strcmp(msg, SharedSecret) == 0)
          ret = 1;
    }
+   if (In)
+      fclose(In);
    dFree(rcline);
    dFree(fname);
    dFree(msg);
@@ -243,7 +245,6 @@
    /* init descriptors and streams */
    dsh->fd_in  = fd_in;
    dsh->fd_out = fd_out;
-   dsh->out = fdopen(fd_out, "w");
 
    /* init buffer */
    dsh->dbuf = dStr_sized_new(8 *1024);
@@ -263,30 +264,91 @@
  */
 int a_Dpip_dsh_write(Dsh *dsh, int flush, const char *Data, int DataSize)
 {
-   int ret = 1;
+   int blocking, old_flags, st, sent = 0, ret = 1;
 
    /* append to buf */
    dStr_append_l(dsh->dbuf, Data, DataSize);
 
-   /* flush data if necessary */
-   if (flush || dsh->dbuf->len >= dsh->flush_sz) {
-      if (dsh->dbuf->len &&
-          fwrite (dsh->dbuf->str, dsh->dbuf->len, 1, dsh->out) != 1) {
-         MSG_ERR("[a_Dpip_dsh_write] %s\n", dStrerror(errno));
+   if (!flush || dsh->dbuf->len == 0)
+      return 0;
+
+   blocking = !(dsh->mode & DPIP_NONBLOCK);
+   if (!blocking) {
+      /* set BLOCKING temporarily... */
+      old_flags = fcntl(dsh->fd_in, F_GETFL);
+      fcntl(dsh->fd_in, F_SETFL, old_flags & ~O_NONBLOCK);
+   }
+
+   while (1) {
+      st = write(dsh->fd_out, dsh->dbuf->str + sent, dsh->dbuf->len - sent);
+      if (st < 0) {
+         if (errno == EINTR) {
+            continue;
+         } else {
+            dsh->status = DPIP_ERROR;
+            break;
+         }
       } else {
-         fflush(dsh->out);
-         dStr_truncate(dsh->dbuf, 0);
-         ret = 0;
+         sent += st;
+         if (sent == dsh->dbuf->len) {
+            dStr_truncate(dsh->dbuf, 0);
+            ret = 0;
+            break;
+         }
       }
+   }
 
-   } else {
-      ret = 0;
+   if (!blocking) {
+      /* restore nonblocking mode */
+      fcntl(dsh->fd_in, F_SETFL, old_flags);
    }
 
    return ret;
 }
 
 /*
+ * Return value: 1..DataSize sent, -1 eagain, or -3 on big Error
+ */
+int a_Dpip_dsh_trywrite(Dsh *dsh, const char *Data, int DataSize)
+{
+   int blocking, old_flags, st, ret = -3, sent = 0;
+
+   blocking = !(dsh->mode & DPIP_NONBLOCK);
+   if (blocking) {
+      /* set NONBLOCKING temporarily... */
+      old_flags = fcntl(dsh->fd_in, F_GETFL);
+      fcntl(dsh->fd_in, F_SETFL, O_NONBLOCK | old_flags);
+   }
+
+   while (1) {
+      st = write(dsh->fd_out, Data + sent, DataSize - sent);
+      if (st < 0) {
+         if (errno == EINTR) {
+            continue;
+         } else if (errno == EAGAIN) {
+            dsh->status = DPIP_EAGAIN;
+            ret = -1;
+            break;
+         } else {
+            MSG_ERR("[a_Dpip_dsh_trywrite] %s\n", dStrerror(errno));
+            dsh->status = DPIP_ERROR;
+            ret = -3;
+            break;
+         }
+      }
+      sent += st;
+      break;
+   }
+
+   if (blocking) {
+      /* restore blocking mode */
+      fcntl(dsh->fd_in, F_SETFL, old_flags);
+   }
+
+   return (st > 0 ? sent : ret);
+}
+
+/*
  * Convenience function.
  */
 int a_Dpip_dsh_write_str(Dsh *dsh, int flush, const char *str)
@@ -303,6 +365,8 @@
    int old_flags, blocking;
    char buf[RBUF_SZ];
 
+   dReturn_if (dsh->status == DPIP_ERROR || dsh->status == DPIP_EOF);
+
    blocking = !(dsh->mode & DPIP_NONBLOCK);
    if (blocking) {
       /* set NONBLOCKING temporarily... */
@@ -320,7 +384,7 @@
             /* no problem, return what we've got so far... */
             dsh->status = DPIP_EAGAIN;
          } else {
-            MSG_ERR("[Dpip_dsh_read] %s\n", dStrerror(errno));
+            MSG_ERR("[Dpip_dsh_read_nb] %s\n", dStrerror(errno));
             dsh->status = DPIP_ERROR;
          }
          break;
@@ -349,6 +413,8 @@
    int old_flags, non_blocking;
    char buf[RBUF_SZ];
 
+   dReturn_if (dsh->status == DPIP_ERROR || dsh->status == DPIP_EOF);
+
    non_blocking = (dsh->mode & DPIP_NONBLOCK);
    if (non_blocking) {
       /* set blocking mode temporarily */
@@ -434,10 +500,20 @@
  */
 void a_Dpip_dsh_close(Dsh *dsh)
 {
+   int st;
+
    /* flush internal buffer */
    a_Dpip_dsh_write(dsh, 1, "", 0);
-   fclose(dsh->out);
-   close(dsh->fd_out);
+
+   /* close fds */
+   while((st = close(dsh->fd_in)) < 0 && errno == EINTR) ;
+   if (st < 0)
+      MSG_ERR("[a_Dpip_dsh_close] close: %s\n", dStrerror(errno));
+   if (dsh->fd_out != dsh->fd_in) {
+      while((st = close(dsh->fd_out)) < 0 && errno == EINTR) ;
+      if (st < 0)
+         MSG_ERR("[a_Dpip_dsh_close] close: %s\n", dStrerror(errno));
+   }
 }
 
 /*
--- a/dpip/dpip.h	Sun Nov 01 16:31:59 2009 -0300
+++ b/dpip/dpip.h	Sun Nov 01 16:31:59 2009 -0300
@@ -68,6 +68,7 @@
 Dsh *a_Dpip_dsh_new(int fd_in, int fd_out, int flush_sz);
 int a_Dpip_dsh_write(Dsh *dsh, int flush, const char *Data, int DataSize);
 int a_Dpip_dsh_write_str(Dsh *dsh, int flush, const char *str);
+int a_Dpip_dsh_trywrite(Dsh *dsh, const char *Data, int DataSize);
 char *a_Dpip_dsh_read_token(Dsh *dsh, int blocking);
 void a_Dpip_dsh_close(Dsh *dsh);
 void a_Dpip_dsh_free(Dsh *dsh);
--- a/src/IO/dpi.c	Sun Nov 01 16:31:59 2009 -0300
+++ b/src/IO/dpi.c	Sun Nov 01 16:31:59 2009 -0300
@@ -423,6 +423,19 @@
 }
 
 /*
+ * Return a socket file descriptor
+ */
+static int Dpi_make_socket_fd()
+{
+   int fd, ret = -1;
+
+   if ((fd = socket(AF_INET, SOCK_STREAM, 0)) != -1) {
+      ret = fd;
+   }
+   return ret;
+}
+
+/*
  * Make a connection test for a IDS.
  * Return: 1 OK, -1 Not working.
  */
@@ -439,7 +452,7 @@
 
    if (Dpi_read_comm_keys(&dpid_port) != -1) {
       sin.sin_port = htons(dpid_port);
-      if ((sock_fd = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
+      if ((sock_fd = Dpi_make_socket_fd()) == -1) {
          MSG("Dpi_check_dpid_ids: sock_fd=%d %s\n", sock_fd, dStrerror(errno));
       } else if (connect(sock_fd, (struct sockaddr *)&sin, sin_sz) == -1) {
          MSG("Dpi_check_dpid_ids: %s\n", dStrerror(errno));
@@ -538,7 +551,7 @@
       sin.sin_family = AF_INET;
       sin.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
       sin.sin_port = htons(dpid_port);
-      if ((sock_fd = socket(AF_INET, SOCK_STREAM, 0)) == -1 ||
+      if ((sock_fd = Dpi_make_socket_fd()) == -1 ||
           connect(sock_fd, (struct sockaddr *)&sin, sin_sz) == -1) {
          MSG("Dpi_get_server_port: %s\n", dStrerror(errno));
       } else {
@@ -613,7 +626,7 @@
    sin.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
    sin.sin_port = htons(dpi_port);
 
-   if ((sock_fd = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
+   if ((sock_fd = Dpi_make_socket_fd()) == -1) {
       perror("[dpi::socket]");
    } else if (connect(sock_fd, (void*)&sin, sizeof(sin)) == -1) {
       err = errno;
--- a/src/auth.c	Sun Nov 01 16:31:59 2009 -0300
+++ b/src/auth.c	Sun Nov 01 16:31:59 2009 -0300
@@ -229,7 +229,7 @@
    set_realm =
       auth_parse->realm == NULL &&
       dStrncasecmp(realm_token,token,token_size) == 0 &&
-      strlen(realm_token) == token_size;
+      strlen(realm_token) == (size_t)token_size;
 
    return Auth_parse_quoted_string(auth_parse, set_realm, auth);
 }