From 1648829c39f793a22b3fd896fbf63ff7bc1e4f67 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 31 Jan 2001 12:39:53 +0000 Subject: [PATCH] Fixed W32 bugs and one major bug which swallowed up some output. --- TODO | 4 +- gpgme/ChangeLog | 10 +++++ gpgme/debug.c | 2 + gpgme/rungpg.c | 5 ++- gpgme/version.c | 8 +++- gpgme/w32-io.c | 114 +++++++++++++++++++++++++++++++----------------- gpgme/wait.c | 68 ++++------------------------- 7 files changed, 103 insertions(+), 108 deletions(-) diff --git a/TODO b/TODO index a829ea0e..c6f65057 100644 --- a/TODO +++ b/TODO @@ -5,6 +5,4 @@ * Allow to use GTK's main loop instead of the select stuff in wait.c -* Remove all that funny exit code handling - we don't need it. - - +* need to close a lot of handles in w32-io.c diff --git a/gpgme/ChangeLog b/gpgme/ChangeLog index 2b8c2370..458b3c97 100644 --- a/gpgme/ChangeLog +++ b/gpgme/ChangeLog @@ -1,3 +1,13 @@ +2001-01-31 Werner Koch + + * wait.c (_gpgme_wait_on_condition): Remove all exit code processing. + (propagate_term_results,clear_active_fds): Removed. + (count_active_fds): Renamed to .. + (count_active_and_thawed_fds): .. this and count only thawed fds. + + * rungpg.c (gpg_colon_line_handler): Return colon.eof and not + status.eof ;-) + 2001-01-30 Werner Koch * w32-io.c (_gpgme_io_spawn): Use the supplied path arg. diff --git a/gpgme/debug.c b/gpgme/debug.c index 7cb5121d..e700405b 100644 --- a/gpgme/debug.c +++ b/gpgme/debug.c @@ -139,6 +139,7 @@ _gpgme_debug_end (void **helper, const char *text) return; _gpgme_debug_add (helper, "%s", text ); + fflush (ctl->fp); /* we need this for the buggy Windoze libc */ rewind (ctl->fp); LOCK (debug_lock); while ( (c=getc (ctl->fp)) != EOF ) { @@ -149,6 +150,7 @@ _gpgme_debug_end (void **helper, const char *text) putc ('\n', stderr); UNLOCK (debug_lock); + fclose (ctl->fp); remove (ctl->fname); xfree (ctl); *helper = NULL; diff --git a/gpgme/rungpg.c b/gpgme/rungpg.c index 8c9d2b77..5250b565 100644 --- a/gpgme/rungpg.c +++ b/gpgme/rungpg.c @@ -1018,7 +1018,8 @@ read_status ( GpgObject gpg ) } } if ( r->code == STATUS_END_STREAM ) { - /* _gpgme_freeze_fd ( ? );*/ + if ( gpg->cmd.used ) + _gpgme_freeze_fd ( gpg->cmd.fd ); } } /* To reuse the buffer for the next line we have to @@ -1066,7 +1067,7 @@ gpg_colon_line_handler ( void *opaque, int pid, int fd ) return 1; } - return gpg->status.eof; + return gpg->colon.eof; } static GpgmeError diff --git a/gpgme/version.c b/gpgme/version.c index 6013d27d..cac19826 100644 --- a/gpgme/version.c +++ b/gpgme/version.c @@ -216,14 +216,14 @@ version_line_handler ( GpgmeCtx c, char *line ) static const char * get_engine_info (void) { - const char *engine_info =NULL; + static const char *engine_info =NULL; GpgmeCtx c = NULL; GpgmeError err = 0; const char *path = NULL; /* FIXME: make sure that only one instance does run */ if (engine_info) - goto leave; + return engine_info; path = _gpgme_get_gpg_path (); err = gpgme_new (&c); @@ -299,3 +299,7 @@ get_engine_info (void) } + + + + diff --git a/gpgme/w32-io.c b/gpgme/w32-io.c index 2ee10bbb..82e8f819 100644 --- a/gpgme/w32-io.c +++ b/gpgme/w32-io.c @@ -58,12 +58,11 @@ struct reader_context_s { DECLARE_LOCK (mutex); int eof; + int eof_shortcut; int error; int error_code; HANDLE have_data_ev; /* manually reset */ - int have_data_flag; /* FIXME: is there another way to check whether - it has been signaled? */ HANDLE have_space_ev; /* auto reset */ size_t readpos, writepos; char buffer[READBUF_SIZE]; @@ -128,10 +127,17 @@ reader (void *arg) DEBUG2 ("reader thread %p: reading %d bytes", c->thread_hd, nbytes ); if ( !ReadFile ( c->file_hd, c->buffer+c->writepos, nbytes, &nread, NULL) ) { - c->error = 1; c->error_code = (int)GetLastError (); - DEBUG2 ("reader thread %p: read error: ec=%d", - c->thread_hd, c->error_code ); + if (c->error_code == ERROR_BROKEN_PIPE ) { + c->eof=1; + DEBUG1 ("reader thread %p: got eof (broken pipe)", + c->thread_hd ); + } + else { + c->error = 1; + DEBUG2 ("reader thread %p: read error: ec=%d", + c->thread_hd, c->error_code ); + } break; } if ( !nread ) { @@ -143,10 +149,11 @@ reader (void *arg) LOCK (c->mutex); c->writepos = (c->writepos + nread) % READBUF_SIZE; - c->have_data_flag = 1; SetEvent (c->have_data_ev); UNLOCK (c->mutex); } + /* indicate that we have an error or eof */ + SetEvent (c->have_data_ev); DEBUG1 ("reader thread %p ended", c->thread_hd ); return 0; @@ -245,34 +252,45 @@ _gpgme_io_read ( int fd, void *buffer, size_t count ) DEBUG0 ( "no reader thread\n"); return -1; } + if (c->eof_shortcut) { + DEBUG1 ("fd %d: EOF (again)", fd ); + return 0; + } LOCK (c->mutex); - if (c->readpos == c->writepos) { /* no data avail */ + if (c->readpos == c->writepos && !c->error) { /*no data avail*/ UNLOCK (c->mutex); DEBUG2 ("fd %d: waiting for data from thread %p", fd, c->thread_hd); WaitForSingleObject (c->have_data_ev, INFINITE); DEBUG2 ("fd %d: data from thread %p available", fd, c->thread_hd); LOCK (c->mutex); - if (c->readpos == c->writepos && !c->eof && !c->error) { - UNLOCK (c->mutex); - if (c->eof) - return 0; - return -1; - } } - + + if (c->readpos == c->writepos || c->error) { + UNLOCK (c->mutex); + c->eof_shortcut = 1; + if (c->eof) { + DEBUG1 ("fd %d: EOF", fd ); + return 0; + } + if (!c->error) { + DEBUG1 ("fd %d: EOF but eof flag not set", fd ); + return 0; + } + DEBUG1 ("fd %d: read error", fd ); + return -1; + } + nread = c->readpos < c->writepos? c->writepos - c->readpos : READBUF_SIZE - c->readpos; if (nread > count) nread = count; memcpy (buffer, c->buffer+c->readpos, nread); c->readpos = (c->readpos + nread) % READBUF_SIZE; - if (c->readpos == c->writepos) { - c->have_data_flag = 0; + if (c->readpos == c->writepos && !c->eof) { ResetEvent (c->have_data_ev); } - if (nread) - SetEvent (c->have_space_ev); + SetEvent (c->have_space_ev); UNLOCK (c->mutex); DEBUG2 ("fd %d: got %d bytes\n", fd, nread ); @@ -287,9 +305,9 @@ _gpgme_io_write ( int fd, const void *buffer, size_t count ) DWORD nwritten; HANDLE h = fd_to_handle (fd); -#warning writing blocks for large counts, so we limit it here. - if (count > 500) - count = 500; + /* writing blocks for large counts, so we limit it here. */ + if (count > 1024) + count = 1024; DEBUG2 ("fd %d: about to write %d bytes\n", fd, (int)count ); if ( !WriteFile ( h, buffer, count, &nwritten, NULL) ) { @@ -587,6 +605,7 @@ _gpgme_io_select ( struct io_select_fd_s *fds, size_t nfds ) { #if 1 HANDLE waitbuf[MAXIMUM_WAIT_OBJECTS]; + int waitidx[MAXIMUM_WAIT_OBJECTS]; int code, nwait; int i, any, any_write; int count; @@ -599,27 +618,26 @@ _gpgme_io_select ( struct io_select_fd_s *fds, size_t nfds ) for ( i=0; i < nfds; i++ ) { if ( fds[i].fd == -1 ) continue; - if ( fds[i].for_read ) { - if ( nwait >= DIM (waitbuf) ) { - DEBUG_END (dbg_help, "oops ]"); - DEBUG0 ("Too many objects for WFMO!" ); - return -1; - } - else { - if ( fds[i].for_read ) { - struct reader_context_s *c = find_reader (fds[i].fd,1); - - if (!c) { - DEBUG1 ("no reader thread for fd %d", fds[i].fd); - } - else { - waitbuf[nwait++] = c->have_data_ev; - } + if ( fds[i].for_read || fds[i].for_write ) { + if ( fds[i].for_read ) { + struct reader_context_s *c = find_reader (fds[i].fd,1); + + if (!c) { + DEBUG1 ("oops: no reader thread for fd %d", fds[i].fd); + } + else { + if ( nwait >= DIM (waitbuf) ) { + DEBUG_END (dbg_help, "oops ]"); + DEBUG0 ("Too many objects for WFMO!" ); + return -1; + } + waitidx[nwait] = i; + waitbuf[nwait++] = c->have_data_ev; } - DEBUG_ADD2 (dbg_help, "%c%d ", - fds[i].for_read? 'r':'w',fds[i].fd ); - any = 1; } + DEBUG_ADD2 (dbg_help, "%c%d ", + fds[i].for_read? 'r':'w',fds[i].fd ); + any = 1; } fds[i].signaled = 0; } @@ -651,7 +669,8 @@ _gpgme_io_select ( struct io_select_fd_s *fds, size_t nfds ) any = 0; for (i=code - WAIT_OBJECT_0; i < nwait; i++ ) { if (WaitForSingleObject ( waitbuf[i], NULL ) == WAIT_OBJECT_0) { - fds[i].signaled = 1; + assert (waitidx[i] >=0 && waitidx[i] < nfds); + fds[waitidx[i]].signaled = 1; any = 1; count++; } @@ -686,6 +705,19 @@ _gpgme_io_select ( struct io_select_fd_s *fds, size_t nfds ) count = -1; } + if ( count ) { + DEBUG_BEGIN (dbg_help, " signaled [ "); + for ( i=0; i < nfds; i++ ) { + if ( fds[i].fd == -1 ) + continue; + if ( (fds[i].for_read || fds[i].for_write) && fds[i].signaled ) { + DEBUG_ADD2 (dbg_help, "%c%d ", + fds[i].for_read? 'r':'w',fds[i].fd ); + } + } + DEBUG_END (dbg_help, "]"); + } + return count; #else /* This is the code we use */ int i, any, count; diff --git a/gpgme/wait.c b/gpgme/wait.c index 71038fb3..c0cfe133 100644 --- a/gpgme/wait.c +++ b/gpgme/wait.c @@ -53,9 +53,6 @@ struct wait_item_s { void *handler_value; int pid; int inbound; /* this is an inbound data handler fd */ - int exited; - int exit_status; - int exit_signal; GpgmeCtx ctx; }; @@ -82,51 +79,20 @@ queue_item_from_context ( GpgmeCtx ctx ) } -static void -propagate_term_results ( const struct wait_item_s *first_q ) -{ - struct wait_item_s *q; - int i; - - for (i=0; i < fd_table_size; i++ ) { - if ( fd_table[i].fd != -1 && (q=fd_table[i].opaque) - && q != first_q && !q->exited - && q->pid == first_q->pid ) { - q->exited = first_q->exited; - q->exit_status = first_q->exit_status; - q->exit_signal = first_q->exit_signal; - } - } -} - static int -count_active_fds ( int pid ) +count_active_and_thawed_fds ( int pid ) { struct wait_item_s *q; int i, count = 0; for (i=0; i < fd_table_size; i++ ) { if ( fd_table[i].fd != -1 && (q=fd_table[i].opaque) - && q->active && q->pid == pid ) + && q->active && !fd_table[i].frozen && q->pid == pid ) count++; } return count; } -static void -clear_active_fds ( int pid ) -{ - struct wait_item_s *q; - int i; - - for (i=0; i < fd_table_size; i++ ) { - if ( fd_table[i].fd != -1 && (q=fd_table[i].opaque) - && q->active && q->pid == pid ) - q->active = 0; - } -} - - /* remove the given process from the queue */ static void remove_process ( int pid ) @@ -187,28 +153,9 @@ _gpgme_wait_on_condition ( GpgmeCtx c, int hang, volatile int *cond ) q = queue_item_from_context ( c ); assert (q); - if (q->exited) { - /* this is the second time we reached this and we got no - * more data from the pipe (which may happen due to buffering). - * Set all FDs inactive. - */ - clear_active_fds (q->pid); - } - else if ( _gpgme_io_waitpid (q->pid, 0, - &q->exit_status, &q->exit_signal)){ - q->exited = 1; - propagate_term_results (q); - } - - if ( q->exited ) { - if ( !count_active_fds (q->pid) ) { - /* Hmmm, as long as we don't have a callback for - * the exit status, we have no use for these - * values and therefore we can remove this from - * the queue */ - remove_process (q->pid); - hang = 0; - } + if ( !count_active_and_thawed_fds (q->pid) ) { + remove_process (q->pid); + hang = 0; } } if (hang) @@ -250,6 +197,7 @@ do_select ( void ) any = 1; if ( q->active && q->handler (q->handler_value, q->pid, fd_table[i].fd ) ) { + DEBUG1 ("setting fd %d inactive", fd_table[i].fd ); q->active = 0; fd_table[i].for_read = 0; fd_table[i].for_write = 0; @@ -334,7 +282,7 @@ _gpgme_freeze_fd ( int fd ) for (i=0; i < fd_table_size; i++ ) { if ( fd_table[i].fd == fd ) { fd_table[i].frozen = 1; - /*fprintf (stderr, "** FD %d frozen\n", fd );*/ + DEBUG1 ("fd %d frozen", fd ); break; } } @@ -350,7 +298,7 @@ _gpgme_thaw_fd ( int fd ) for (i=0; i < fd_table_size; i++ ) { if ( fd_table[i].fd == fd ) { fd_table[i].frozen = 0; - /*fprintf (stderr, "** FD %d thawed\n", fd );*/ + DEBUG1 ("fd %d thawed", fd ); break; } }