[svn] dirmngr - r322 - trunk/src

svn author wk cvs at cvs.gnupg.org
Tue Aug 4 15:49:18 CEST 2009


Author: wk
Date: 2009-08-04 15:49:17 +0200 (Tue, 04 Aug 2009)
New Revision: 322

Modified:
   trunk/src/ChangeLog
   trunk/src/ldap.c
Log:
Improved logging from the ldap wrapper.


Modified: trunk/src/ChangeLog
===================================================================
--- trunk/src/ChangeLog	2009-07-31 13:36:49 UTC (rev 321)
+++ trunk/src/ChangeLog	2009-08-04 13:49:17 UTC (rev 322)
@@ -1,3 +1,10 @@
+2009-08-04  Werner Koch  <wk at g10code.com>
+
+	* ldap.c (ldap_wrapper_thread): Factor some code out to ...
+	(read_log_data): ... new.  Close the log fd on error.
+	(ldap_wrapper_thread): Delay cleanup until the log fd is closed.
+	(SAFE_PTH_CLOSE): New.  Use it instead of pth_close.
+
 2009-07-31  Werner Koch  <wk at g10code.com>
 
 	* server.c (cmd_loadcrl): Add option --url.

Modified: trunk/src/ldap.c
===================================================================
--- trunk/src/ldap.c	2009-07-31 13:36:49 UTC (rev 321)
+++ trunk/src/ldap.c	2009-08-04 13:49:17 UTC (rev 322)
@@ -107,14 +107,17 @@
 /* We need to know whether we are shutting down the process */
 static int shutting_down;
 
+/* Close the pth file descriptor FD and set it to -1.  */
+#define SAFE_PTH_CLOSE(fd) \
+  do { int _fd = fd; if (_fd != -1) { pth_close (_fd); fd = -1;} } while (0)
 
+
 /* Prototypes.  */
 static gpg_error_t read_buffer (ksba_reader_t reader,
                                 unsigned char *buffer, size_t count);
 
 
 
-
 
 /* Add HOST and PORT to our list of LDAP servers.  Fixme: We should
    better use an extra list of servers. */
@@ -173,10 +176,8 @@
       dirmngr_release_process (ctx->pid);
     }
   ksba_reader_release (ctx->reader);
-  if (ctx->fd != -1)
-    pth_close (ctx->fd);
-  if (ctx->log_fd != -1)
-    pth_close (ctx->log_fd);
+  SAFE_PTH_CLOSE (ctx->fd);
+  SAFE_PTH_CLOSE (ctx->log_fd);
   if (ctx->log_ev)
     pth_event_free (ctx->log_ev, PTH_FREE_THIS);
   xfree (ctx->line);
@@ -243,17 +244,47 @@
 }
 
 
+/* Read data from the log stream.  Returns true if the log stream
+   indicated EOF or error.  */
+static int
+read_log_data (struct wrapper_context_s *ctx)
+{
+  int n;
+  char line[256];
 
+  /* We must use the pth_read function for pipes, always.  */
+  do 
+    n = pth_read (ctx->log_fd, line, sizeof line - 1);
+  while (n < 0 && errno == EINTR);
+
+  if (n <= 0) /* EOF or error. */
+    {
+      if (n < 0)
+        log_error (_("error reading log from ldap wrapper %d: %s\n"),
+                   ctx->pid, strerror (errno));
+      print_log_line (ctx, NULL);
+      SAFE_PTH_CLOSE (ctx->log_fd);
+      pth_event_free (ctx->log_ev, PTH_FREE_THIS);
+      ctx->log_ev = NULL;
+      return 1;
+    }
+
+  line[n] = 0;
+  print_log_line (ctx, line);
+  if (ctx->stamp != (time_t)(-1))
+    ctx->stamp = time (NULL);
+  return 0;
+}
+
+
 /* This function is run by a separate thread to maintain the list of
    wrappers and to log error messages from these wrappers. */
 void *
 ldap_wrapper_thread (void *dummy)
 {
   int nfds;
-  int n;
   struct wrapper_context_s *ctx;
   struct wrapper_context_s *ctx_prev;
-  char line[256];
   time_t current_time;
 
   (void)dummy;
@@ -310,33 +341,8 @@
           if (nfds && ctx->log_fd != -1
 	      && pth_event_status (ctx->log_ev) == PTH_STATUS_OCCURRED)
             {
-              /* We must use the pth_read function for pipes, always.  */
-              do 
-                n = pth_read (ctx->log_fd, line, sizeof line - 1);
-              while (n < 0 && errno == EINTR);
-              if (n < 0)
-                {
-                  print_log_line (ctx, NULL);
-                  log_error (_("error reading log from ldap wrapper %d: %s\n"),
-                             ctx->pid, strerror (errno));
-                  any_action = 1;
-                }
-              else if (!n) /* EOF */
-                {
-                  print_log_line (ctx, NULL);
-                  pth_close (ctx->log_fd);
-                  ctx->log_fd = -1;
-                  pth_event_free (ctx->log_ev, PTH_FREE_THIS);
-                  ctx->log_ev = NULL;
-                  any_action = 1;
-                }
-              else 
-                {
-                  line[n] = 0;
-                  print_log_line (ctx, line);
-                  if (ctx->stamp != (time_t)(-1))
-                    ctx->stamp = time (NULL);
-                }
+              if (read_log_data (ctx))
+                any_action = 1;
             }
 
           /* Check whether the process is still running.  */
@@ -372,6 +378,9 @@
                 ctx->stamp = (time_t)(-1);
               log_info (_("ldap wrapper %d stalled - killing\n"),
                         (int)ctx->pid);
+              /* We need to close the log fd because the cleanup loop
+                 waits for it.  */
+              SAFE_PTH_CLOSE (ctx->log_fd);
               any_action = 1;
             }
         }
@@ -397,7 +406,8 @@
          is not anymore in use or we are in shutdown state.  */
      again:
       for (ctx_prev=NULL, ctx=wrapper_list; ctx; ctx_prev=ctx, ctx=ctx->next)
-        if (ctx->ready && (!ctx->reader || shutting_down))
+        if (ctx->ready 
+            && ((ctx->log_fd == -1 && !ctx->reader) || shutting_down))
           {
             if (ctx_prev)
               ctx_prev->next = ctx->next;
@@ -431,7 +441,6 @@
 ldap_wrapper_release_context (ksba_reader_t reader)
 {
   struct wrapper_context_s *ctx;
-  int fd;
 
   if (!reader )
     return;
@@ -445,14 +454,9 @@
                     (int)ctx->pid, (int)ctx->printable_pid,
                     ctx->reader,
                     ctx->ctrl, ctx->ctrl? ctx->ctrl->refcount:0);
-        
+
         ctx->reader = NULL;
-        if (ctx->fd != -1)
-          {
-            fd = ctx->fd;
-            ctx->fd = -1;
-            pth_close (fd);
-          }
+        SAFE_PTH_CLOSE (ctx->fd);
         if (ctx->ctrl)
           {
             ctx->ctrl->refcount--;
@@ -527,8 +531,7 @@
           if (err)
             {
               ctx->fd_error = err;
-              pth_close (ctx->fd);
-              ctx->fd = -1;
+              SAFE_PTH_CLOSE (ctx->fd);
               if (evt)
                 pth_event_free (evt, PTH_FREE_THIS);
               return -1;
@@ -538,8 +541,7 @@
       else if (n < 0)
         {
           ctx->fd_error = gpg_error_from_errno (errno);
-          pth_close (ctx->fd);
-          ctx->fd = -1;
+          SAFE_PTH_CLOSE (ctx->fd);
           if (evt)
             pth_event_free (evt, PTH_FREE_THIS);
           return -1;




More information about the Gnupg-commits mailing list