[svn] GpgOL - r244 - in trunk: . src

svn author wk cvs at cvs.gnupg.org
Fri Apr 4 16:14:28 CEST 2008


Author: wk
Date: 2008-04-04 16:14:27 +0200 (Fri, 04 Apr 2008)
New Revision: 244

Modified:
   trunk/ChangeLog
   trunk/NEWS
   trunk/configure.ac
   trunk/src/ChangeLog
   trunk/src/common.c
   trunk/src/engine-assuan.c
   trunk/src/engine.c
   trunk/src/main.c
Log:
Fix sign+encr problem.


[The diff below has been truncated]

Modified: trunk/ChangeLog
===================================================================
--- trunk/ChangeLog	2008-04-02 16:15:24 UTC (rev 243)
+++ trunk/ChangeLog	2008-04-04 14:14:27 UTC (rev 244)
@@ -1,3 +1,11 @@
+2008-04-04  Werner Koch  <wk at g10code.com>
+
+	* Release 0.10.11.
+
+2008-04-02  Werner Koch  <wk at g10code.com>
+
+	* Release 0.10.10.
+
 2008-04-01  Werner Koch  <wk at g10code.com>
 
 	* configure.ac (AC_INIT): Fix quoting.

Modified: trunk/src/ChangeLog
===================================================================
--- trunk/src/ChangeLog	2008-04-02 16:15:24 UTC (rev 243)
+++ trunk/src/ChangeLog	2008-04-04 14:14:27 UTC (rev 244)
@@ -1,3 +1,17 @@
+2008-04-04  Werner Koch  <wk at g10code.com>
+
+	* engine-assuan.c (worker_start_read, worker_check_read): Factor
+	common code out to ..
+	(write_to_callback): .. new.
+	(async_worker_thread): Better comments and minor changes.
+	(enqueue_callback): Add arg INACTIVE.
+	(set_items_active): New.
+	(start_command): Set items active.
+	(op_assuan_encrypt): Create input and output items as inactive.
+	(async_worker_thread): Handle the inactive flag.
+
+	* common.c (gpgol_spawn_detached): Do not inherit handles.
+
 2008-04-02  Werner Koch  <wk at g10code.com>
 
 	* engine-assuan.c (destroy_command): Add arg FORCE.

Modified: trunk/NEWS
===================================================================
--- trunk/NEWS	2008-04-02 16:15:24 UTC (rev 243)
+++ trunk/NEWS	2008-04-04 14:14:27 UTC (rev 244)
@@ -1,3 +1,9 @@
+Noteworthy changes for version 0.10.11 (2008-04-04)
+===================================================
+
+ * Fixed a performavce problem with signed+encrypted.
+
+
 Noteworthy changes for version 0.10.10 (2008-04-02)
 ===================================================
 

Modified: trunk/configure.ac
===================================================================
--- trunk/configure.ac	2008-04-02 16:15:24 UTC (rev 243)
+++ trunk/configure.ac	2008-04-04 14:14:27 UTC (rev 244)
@@ -16,7 +16,7 @@
 # Remember to change the version number immediately *after* a release.
 # Set my_issvn to "yes" for non-released code.  Remember to run an
 # "svn up" and "autogen.sh" right before creating a distribution.
-m4_define([my_version], [0.10.10])
+m4_define([my_version], [0.10.11])
 m4_define([my_issvn], [no])
 
 m4_define([svn_revision], m4_esyscmd([echo -n $( (svn info 2>/dev/null \

Modified: trunk/src/common.c
===================================================================
--- trunk/src/common.c	2008-04-02 16:15:24 UTC (rev 243)
+++ trunk/src/common.c	2008-04-04 14:14:27 UTC (rev 244)
@@ -31,6 +31,7 @@
 #endif
 #include <time.h>
 #include <fcntl.h>
+#include <ctype.h>
 
 #include "common.h"
 
@@ -788,7 +789,7 @@
 }
 
 
-/* Fork and exec the program gioven in CMDLINE with /dev/null as
+/* Fork and exec the program given in CMDLINE with /dev/null as
    stdin, stdout and stderr.  Returns 0 on success.  */
 int
 gpgol_spawn_detached (const char *cmdline)
@@ -818,7 +819,7 @@
                       cmdline_copy,  /* Command line arguments.  */
                       &sec_attr,     /* Process security attributes.  */
                       &sec_attr,     /* Thread security attributes.  */
-                      TRUE,          /* Inherit handles.  */
+                      FALSE,          /* Inherit handles.  */
                       cr_flags,      /* Creation flags.  */
                       NULL,          /* Environment.  */
                       NULL,          /* Use current drive/directory.  */

Modified: trunk/src/engine-assuan.c
===================================================================
--- trunk/src/engine-assuan.c	2008-04-02 16:15:24 UTC (rev 243)
+++ trunk/src/engine-assuan.c	2008-04-04 14:14:27 UTC (rev 244)
@@ -99,11 +99,12 @@
   gpgme_data_t data; /* The data object we write to or read from.  */
   int writing;       /* If true we are going to write to HD.  */
   HANDLE hd;         /* The handle we read from or write to.  */
+  int inactive;      /* If set, the handle is not yet active.  */
   int io_pending;    /* I/O is still pending.  The value is the number
                         of bytes to be written or the size of the
                         buffer given to ReadFile. */
   int got_ready;     /* Operation finished.  */
-  int delayed_ready; /* Ready but delayed to to a missing prerequesite.  */
+  int delayed_ready; /* Ready but delayed due to a missing prerequesite.  */
   int got_error;     /* An error as been encountered.  */
   int aborting;      /* Set to true after a CancelIO has been issued.  */
   void (*finalize)(work_item_t); /* Function called immediately before
@@ -672,13 +673,49 @@
 
 
 
+/* Helper to write to the callback.  */
+static void
+write_to_callback (work_item_t item, DWORD nbytes)
+{
+  int nwritten;
+
+  if (!nbytes)
+    {
+      /* (With overlapped, EOF is not indicated by NBYTES==0.)  */
+      log_error ("%s:%s: [%s:%p] short read (0 bytes)",
+                 SRCNAME, __func__, item->name, item->hd);
+    }
+  else
+    {
+      assert (nbytes > 0);
+      nwritten = gpgme_data_write (item->data, item->buffer, nbytes);
+      if (nwritten < 0)
+        {
+          log_error ("%s:%s: [%s:%p] error writing to callback: %s",
+                     SRCNAME, __func__, item->name, item->hd,strerror (errno));
+          item->got_error = 1;
+        }
+      else if (nwritten < nbytes)
+        {
+          log_error ("%s:%s: [%s:%p] short write to callback (%d of %lu)",
+                     SRCNAME, __func__, item->name, item->hd, nwritten,nbytes);
+          item->got_error = 1;
+        }
+      else
+        {
+          if (debug_ioworker)
+            log_debug ("%s:%s: [%s:%p] wrote %d bytes to callback",
+                       SRCNAME, __func__, item->name, item->hd, nwritten);
+        }
+    }
+}
+
 /* Helper for async_worker_thread.  Returns true if the item's handle
    needs to be put on the wait list.  This is called with the worker
    mutex hold. */
 static int
 worker_start_read (work_item_t item)
 {
-  int nwritten;
   DWORD nbytes;
   int retval = 0;
 
@@ -687,34 +724,7 @@
   if (ReadFile (item->hd, item->buffer, sizeof item->buffer,
                 &nbytes, &item->ov) )
     {
-      /* (With overlapped, EOF is not indicated by NBYTES==0.)  */
-      if (!nbytes)
-        log_error ("%s:%s: [%s:%p] short read (0 bytes)",
-                   SRCNAME, __func__, item->name, item->hd);
-      else
-        {
-          nwritten = gpgme_data_write (item->data, item->buffer, nbytes);
-          if (nwritten < 0)
-            {
-              log_error ("%s:%s: [%s:%p] writing to callback failed: %s",
-                         SRCNAME, __func__, item->name, item->hd,
-                         strerror (errno));
-              item->got_error = 1;
-            }
-          else if (nwritten < nbytes)
-            {
-              log_error ("%s:%s: [%s:%p] short write to callback (%d of %lu)",
-                         SRCNAME, __func__, item->name, item->hd,
-                         nwritten, nbytes);
-              item->got_error = 1;
-            }
-          else
-            {
-              if (debug_ioworker)
-                log_debug ("%s:%s: [%s:%p] wrote %d bytes to callback", 
-                           SRCNAME, __func__, item->name, item->hd, nwritten);
-            }
-        }
+      write_to_callback (item, nbytes);
       retval = 1;
     }
   else 
@@ -753,34 +763,7 @@
 static void
 worker_check_read (work_item_t item, DWORD nbytes)
 {
-  int nwritten;
-
-  if (!nbytes)
-    log_error ("%s:%s: [%s:%p] short read (0 bytes)",
-               SRCNAME, __func__, item->name, item->hd);
-  else
-    {
-      assert (nbytes > 0);
-      nwritten = gpgme_data_write (item->data, item->buffer, nbytes);
-      if (nwritten < 0)
-        {
-          log_error ("%s:%s: [%s:%p] error writing to callback: %s",
-                     SRCNAME, __func__, item->name, item->hd,strerror (errno));
-          item->got_error = 1;
-        }
-      else if (nwritten < nbytes)
-        {
-          log_error ("%s:%s: [%s:%p] short write to callback (%d of %lu)",
-                     SRCNAME, __func__, item->name, item->hd, nwritten,nbytes);
-          item->got_error = 1;
-        }
-      else
-        {
-          if (debug_ioworker)
-            log_debug ("%s:%s: [%s:%p] wrote %d bytes to callback",
-                       SRCNAME, __func__, item->name, item->hd, nwritten);
-        }
-    }
+  write_to_callback (item, nbytes);
 }
 
 
@@ -795,23 +778,26 @@
   DWORD nbytes;
   int retval = 0;
 
+  assert (!item->io_pending);
+
   /* Read from the callback and the write to the handle.  The gpgme
      callback is expected to never block.  */
   nread = gpgme_data_read (item->data, item->buffer, sizeof item->buffer);
-  if (nread < 0 && errno == EAGAIN)
-    switch_threads (item);
-  else
-    clear_switch_threads (item);
   if (nread < 0)
     {
       if (errno == EAGAIN)
         {
-/*           log_debug ("%s:%s: [%s:%p] ignoring EAGAIN from callback", */
-/*                      SRCNAME, __func__, item->name, item->hd); */
+          /* EAGAIN from the callback.  That means that data is
+             currently not available.  */
+          if (debug_ioworker_extra)
+            log_debug ("%s:%s: [%s:%p] EAGAIN received from callback",
+                       SRCNAME, __func__, item->name, item->hd);
+          switch_threads (item);
           retval = 1;
         }
       else
         {
+          clear_switch_threads (item);
           log_error ("%s:%s: [%s:%p] error reading from callback: %s",
                      SRCNAME, __func__, item->name, item->hd,strerror (errno));
           item->got_error = 1;
@@ -819,14 +805,15 @@
     }
   else if (!nread)
     {
+      clear_switch_threads (item);
       if (debug_ioworker)
         log_debug ("%s:%s: [%s:%p] EOF received from callback",
                    SRCNAME, __func__, item->name, item->hd);
       item->got_ready = 1;
-      retval = 1;
     }
   else 
     {                  
+      clear_switch_threads (item);
       if (WriteFile (item->hd, item->buffer, nread, &nbytes, &item->ov))
         {
           if (nbytes < nread)
@@ -841,7 +828,7 @@
                 log_debug ("%s:%s: [%s:%p] wrote %lu bytes", 
                            SRCNAME, __func__, item->name, item->hd, nbytes);
             }
-          retval = 1;
+          retval = 1; /* Keep on waiting for space in the pipe.  */
         }
       else 
         {
@@ -849,11 +836,12 @@
 
           if (syserr == ERROR_IO_PENDING)
             {
+              /* This is the common case.  Start the async I/O.  */
               if (debug_ioworker)
                 log_debug ("%s:%s: [%s:%p] io(write) pending (%d bytes)",
                            SRCNAME, __func__, item->name, item->hd, nread);
               item->io_pending = nread;
-              retval = 1;
+              retval = 1; /* Need to wait for the I/O to complete.  */
             }
           else
             {
@@ -905,12 +893,17 @@
 
   for (;;)
     {
-      /* Process our queue and fire up async I/O requests.  */
+      /* 
+         Step 1: Walk our queue and fire up async I/O requests.  
+       */
       if (debug_ioworker_extra)
-        log_debug ("%s:%s: processing work queue", SRCNAME, __func__);
+        log_debug ("%s:%s: step 1 - scanning work queue", SRCNAME, __func__);
       EnterCriticalSection (&work_queue_lock);
+      
+      /* We always need to wait on the the work queue event.  */
       hdarraylen = 0;
       hdarray[hdarraylen++] = work_queue_event;
+
       count = 0;
       any_ready = 0;
       for (item = work_queue; item; item = item->next)
@@ -934,8 +927,15 @@
                            SRCNAME, __func__, item->name, item->hd);
               continue;
             }
-          
-          if (item->io_pending)
+
+          /* Decide whether we need to wait for this item.  This is
+             the case if the previous WriteFile or ReadFile indicated
+             that I/O is pending or if the worker_start_foo function
+             indicated that we should wait.  Put handles we want to
+             wait upon into HDARRAY. */ 
+          if (item->inactive)
+            addit = 0;
+          else if (item->io_pending)
             addit = 1;
           else if (item->writing)
             addit = worker_start_write (item);
@@ -945,26 +945,47 @@
           if (addit)
             {
               hdarray[hdarraylen++] = item->hd;
-              item->waiting = 1; /* Just for the trace output.  */
+              item->waiting = 1; /* Only required for debugging.  */
             }
+
+          /* Set a flag if this work item is ready or got an error.  */
           if (!item->delayed_ready && (item->got_error || item->got_ready))
             any_ready = 1;
         }
       LeaveCriticalSection (&work_queue_lock);
 
+      /*
+         Step 2: Wait for events or handle activitity.
+       */
       if (any_ready)
         {
+          /* There is at least one work item which is ready or got an
+             error.  Skip the wait step so that we process it
+             immediately.  */
           if (debug_ioworker_extra)
-            log_debug ("%s:%s: %d items in queue; skipping wait", 
+            log_debug ("%s:%s: step 2 - %d items in queue; skipping wait", 
                        SRCNAME, __func__, count);
         }
       else
         {
-          /* First process any window messages of this thread.  Do
+          if (debug_ioworker_extra)
+            {
+              log_debug ("%s:%s: step 2 - "
+                         "%d items in queue; waiting for %d items:",
+                         SRCNAME, __func__, count, hdarraylen-1);
+              for (item = work_queue; item; item = item->next)
+                {
+                  if (item->waiting)
+                    log_debug ("%s:%s: [%s:%p]",
+                               SRCNAME, __func__, item->name, item->hd);
+                }
+            }
+          /* [Currently not used]
+             First process any window messages of this thread.  Do
              this before wating so that the message queue is cleared
              before waiting and we don't get stucked due to messages
              not removed.  We need to process the message queue also
-             after the wait becuase we will only get to here if there
+             after the wait because we will only get to here if there
              is actual ui-server work to be done but some messages
              might still be in the queue.  */
 /*           { */
@@ -977,22 +998,14 @@
 /*               } */
 /*           } */
 
-          if (debug_ioworker_extra)
-            {
-              log_debug ("%s:%s: %d items in queue; waiting for %d items:",
-                         SRCNAME, __func__, count, hdarraylen-1);
-              for (item = work_queue; item; item = item->next)
-                {
-                  if (item->waiting)
-                    log_debug ("%s:%s: [%s:%p]",
-                               SRCNAME, __func__, item->name, item->hd);
-                }
-            }
           n = WaitForMultipleObjects (hdarraylen, hdarray, FALSE, INFINITE);
 /*           n = MsgWaitForMultipleObjects (hdarraylen, hdarray, FALSE, */
 /*                                          INFINITE, QS_ALLEVENTS); */
           if (n == WAIT_FAILED)
             {
+              /* The WFMO failed.  This is an error; to help debugging
+                 we now print information about all the handles we
+                 wanted to wait upon.  */
               int i;
               DWORD hdinfo;
                   
@@ -1013,14 +1026,14 @@
           else if (n >= 0 && n < hdarraylen)
             {
               if (debug_ioworker_extra)
-                log_debug ("%s:%s: WFMO succeeded (res=%d)",
-                           SRCNAME,__func__, n);
+                log_debug ("%s:%s: WFMO succeeded (res=%d, hd=%p)",
+                           SRCNAME, __func__, n, hdarray[n]);
             }
           else if (n == hdarraylen)
             {
               if (debug_ioworker_extra)
-                log_debug ("%s:%s: WFMO succeeded - MSGEVENT (res=%d)",
-                           SRCNAME,__func__, n);
+                log_debug ("%s:%s: WFMO succeeded (res=%d, msgevent)",
+                           SRCNAME, __func__, n);
             }
           else
             {
@@ -1028,7 +1041,8 @@
               Sleep (1000);
             }
 
-          /* Try to process the message queue.  */
+          /* [Currently not used] 
+             Try to process the message queue.  */
 /*           { */
 /*             MSG msg; */
             
@@ -1038,20 +1052,29 @@
 /*                 DispatchMessage (&msg); */
 /*               } */
 /*           } */
-
         }
 
-
-      /* Handle completion status.  */
+      /*
+         Step 3: Handle I/O completion status.
+       */
       EnterCriticalSection (&work_queue_lock);
       if (debug_ioworker_extra)
-        log_debug ("%s:%s: checking completion states", SRCNAME, __func__);
+        log_debug ("%s:%s: step 3 - checking completion states", 
+                   SRCNAME, __func__);
       for (item = work_queue; item; item = item->next)
         {
-          if (!item->io_pending)
-            ;
+          if (!item->io_pending || item->inactive)
+            {
+              /* No I/O is pending for that item, thus there is no
+                 need to check a completion status.  */
+            }
           else if (GetOverlappedResult (item->hd, &item->ov, &nbytes, FALSE))
             {
+              /* An overlapped I/O result is available.  Check that
+                 the returned number of bytes are plausible and clear
+                 the I/O pending flag of this work item.  For a a read
+                 item worker_check_read forwards the received data to
+                 the caller.  */
               if (item->writing)
                 worker_check_write (item, nbytes);
               else
@@ -1060,32 +1083,33 @@
             }
           else 
             {
+              /* Some kind of error occured: Set appropriate
+                 flags.  */
               int syserr = GetLastError ();
               if (syserr == ERROR_IO_INCOMPLETE)
-                ;
-              else if (!item->writing && syserr == ERROR_HANDLE_EOF)
                 {
-                  /* Got EOF.  */
-                  if (debug_ioworker)
-                    log_debug ("%s:%s: [%s:%p] EOF received",
-                               SRCNAME, __func__, item->name, item->hd);
-                  item->io_pending = 0;
-                  item->got_ready = 1;
+                  /* This is a common case, the I/O has not yet
+                     completed for this work item.  No need for any
+                     action. */
                 }
-              else if (!item->writing && syserr == ERROR_BROKEN_PIPE)
+              else if (!item->writing && (syserr == ERROR_HANDLE_EOF
+                                          || syserr == ERROR_BROKEN_PIPE) )
                 {
                   /* Got EOF.  */
                   if (debug_ioworker)
-                    log_debug ("%s:%s: [%s:%p] EOF (broken pipe) received",
-                               SRCNAME, __func__, item->name, item->hd);
+                    log_debug ("%s:%s: [%s:%p] EOF%s received",
+                               SRCNAME, __func__, item->name, item->hd,
+                               syserr==ERROR_BROKEN_PIPE?" (broken pipe)":"");
                   item->io_pending = 0;
                   item->got_ready = 1;
                 }
               else
                 {
+                  /* Something went wrong.  We better cancel the I/O. */
                   log_error_w32 (syserr,
                                  "%s:%s: [%s:%p] GetOverlappedResult failed",
                                  SRCNAME, __func__, item->name, item->hd);
+                  item->io_pending = 0;
                   item->got_error = 1;
                   if (!item->aborting)
                     {




More information about the Gnupg-commits mailing list