diff options
Diffstat (limited to '')
| -rw-r--r-- | gpgme/ChangeLog | 9 | ||||
| -rw-r--r-- | gpgme/engine-gpgsm.c | 21 | ||||
| -rw-r--r-- | gpgme/priv-io.h | 8 | ||||
| -rw-r--r-- | gpgme/w32-glib-io.c | 59 | ||||
| -rw-r--r-- | gpgme/w32-qt-io.cpp | 9 |
5 files changed, 61 insertions, 45 deletions
diff --git a/gpgme/ChangeLog b/gpgme/ChangeLog index fe47cff9..b519105a 100644 --- a/gpgme/ChangeLog +++ b/gpgme/ChangeLog @@ -1,3 +1,12 @@ +2007-10-02 Marcus Brinkmann <[email protected]> + + * priv-io.h, engine-gpgsm.c: Add comments. + * w32-qt-io.cpp (_gpgme_io_select): Remove code handling frozen FDs. + * w32-glib-io.c (_gpgme_io_close): Always dereference the channel, + even if not primary. + (_gpgme_io_dup): Acquire a reference. Replace unused + implementation by assertion. + 2007-09-28 Werner Koch <[email protected]> * engine-gpgsm.c (iocb_data_t): Add SERVER_FD_STR. diff --git a/gpgme/engine-gpgsm.c b/gpgme/engine-gpgsm.c index bae06ef0..f024916a 100644 --- a/gpgme/engine-gpgsm.c +++ b/gpgme/engine-gpgsm.c @@ -54,7 +54,7 @@ typedef struct void *data; /* Handler-specific data. */ void *tag; /* ID from the user for gpgme_remove_io_callback. */ char server_fd_str[15]; /* Same as SERVER_FD but as a string. We - need this because _gpgme_io_fdstr can't + need this because _gpgme_io_fd2str can't be used on a closed descriptor. */ } iocb_data_t; @@ -530,8 +530,9 @@ gpgsm_new (void **engine, const char *file_name, const char *home_dir) #endif leave: - /* Close the server ends of the pipes. Our ends are closed in - gpgsm_release(). */ + /* Close the server ends of the pipes (because of this, we must use + the stored server_fd_str in the function start). Our ends are + closed in gpgsm_release(). */ #if !USE_DESCRIPTOR_PASSING if (gpgsm->input_cb.server_fd != -1) _gpgme_io_close (gpgsm->input_cb.server_fd); @@ -1013,11 +1014,15 @@ start (engine_gpgsm_t gpgsm, const char *command) if (nfds < 1) return gpg_error (GPG_ERR_GENERAL); /* FIXME */ - /* We duplicate the file descriptor, so we can close it without - disturbing assuan. Alternatively, we could special case - status_fd and register/unregister it manually as needed, but this - increases code duplication and is more complicated as we can not - use the close notifications etc. */ + /* We "duplicate" the file descriptor, so we can close it here (we + can't close fdlist[0], as that is closed by libassuan, and + closing it here might cause libassuan to close some unrelated FD + later). Alternatively, we could special case status_fd and + register/unregister it manually as needed, but this increases + code duplication and is more complicated as we can not use the + close notifications etc. A third alternative would be to let + Assuan know that we closed the FD, but that complicates the + Assuan interface. */ gpgsm->status_cb.fd = _gpgme_io_dup (fdlist[0]); if (gpgsm->status_cb.fd < 0) diff --git a/gpgme/priv-io.h b/gpgme/priv-io.h index a488acb0..5841dc92 100644 --- a/gpgme/priv-io.h +++ b/gpgme/priv-io.h @@ -64,7 +64,13 @@ int _gpgme_io_select (struct io_select_fd_s *fds, size_t nfds, int nonblock); line that the child process expects. */ int _gpgme_io_fd2str (char *buf, int buflen, int fd); -/* Like dup(). */ +/* Duplicate a file descriptor. This is more restrictive than dup(): + it assumes that the resulting file descriptors are essentially + co-equal (for example, no private offset), which is true for pipes + and sockets (but not files) under Unix with the standard dup() + function. Basically, this function is used to reference count the + status output file descriptor shared between GPGME and libassuan + (in engine-gpgsm.c). */ int _gpgme_io_dup (int fd); #endif /* IO_H */ diff --git a/gpgme/w32-glib-io.c b/gpgme/w32-glib-io.c index faf31a62..3204b578 100644 --- a/gpgme/w32-glib-io.c +++ b/gpgme/w32-glib-io.c @@ -81,7 +81,23 @@ static struct { GIOChannel *chan; - int primary; /* Set if CHAN is the one we used to create the channel. */ + /* The boolean PRIMARY is true if this file descriptor caused the + allocation of CHAN. Only then should CHAN be destroyed when this + FD is closed. This, together with the fact that dup'ed file + descriptors are closed before the file descriptors from which + they are dup'ed are closed, ensures that CHAN is always valid, + and shared among all file descriptors refering to the same + underlying object. + + The logic behind this is that there is only one reason for us to + dup file descriptors anyway: to allow simpler book-keeping of + file descriptors shared between GPGME and libassuan, which both + want to close something. Using the same channel for these + duplicates works just fine (and in fact, using different channels + does not work because the W32 backend in glib does not support + that: One would end up with several competing reader/writer + threads. */ + int primary; } giochannel_table[MAX_SLAFD]; @@ -306,10 +322,11 @@ _gpgme_io_close (int fd) if (giochannel_table[fd].chan) { if (giochannel_table[fd].primary) - { - g_io_channel_shutdown (giochannel_table[fd].chan, 1, NULL); - g_io_channel_unref (giochannel_table[fd].chan); - } + g_io_channel_shutdown (giochannel_table[fd].chan, 1, NULL); + else + _close (fd); + + g_io_channel_unref (giochannel_table[fd].chan); giochannel_table[fd].chan = NULL; } else @@ -731,40 +748,24 @@ _gpgme_io_dup (int fd) TRACE_BEG1 (DEBUG_SYSIO, "_gpgme_io_dup", fd, "dup (%d)", fd); - newfd =_dup (fd); + newfd = _dup (fd); if (newfd == -1) return TRACE_SYSRES (-1); if (newfd < 0 || newfd >= MAX_SLAFD) { - /* New fd won't fit into our table. */ + /* New FD won't fit into our table. */ _close (newfd); errno = EIO; return TRACE_SYSRES (-1); } + assert (giochannel_table[newfd].chan == NULL); chan = find_channel (fd, 0); - if (!chan) - { - /* No channel exists for the original fd, thus we create one for - our new fd. */ - if ( !find_channel (newfd, 1) ) - { - _close (newfd); - errno = EIO; - return TRACE_SYSRES (-1); - } - } - else - { - /* There is already a channel for the original one. Copy that - channel into a new table entry unless we already did so. */ - if ( !giochannel_table[newfd].chan) - { - giochannel_table[newfd].chan = chan; - giochannel_table[newfd].primary = 0; - } - assert (giochannel_table[newfd].chan == chan); - } + assert (chan); + + g_io_channel_ref (chan); + giochannel_table[newfd].chan = chan; + giochannel_table[newfd].primary = 0; return TRACE_SYSRES (newfd); } diff --git a/gpgme/w32-qt-io.cpp b/gpgme/w32-qt-io.cpp index faa9123e..c131f17c 100644 --- a/gpgme/w32-qt-io.cpp +++ b/gpgme/w32-qt-io.cpp @@ -1,4 +1,4 @@ -/* w32-glib-io.c - W32 Glib I/O functions +/* w32-qt-io.c - W32 Glib I/O functions Copyright (C) 2000 Werner Koch (dd9jn) Copyright (C) 2001, 2002, 2004, 2005, 2007 g10 Code GmbH @@ -583,12 +583,7 @@ _gpgme_io_select (struct io_select_fd_s *fds, size_t nfds, int nonblock) { fds[i].signaled = 0; } - else if (fds[i].frozen) - { - TRACE_ADD1 (dbg_help, "f0x%x ", fds[i].fd); - fds[i].signaled = 0; - } - else if (fds[i].for_read ) + else if (fds[i].for_read) { const QIODevice * const chan = find_channel (fds[i].fd, 0); assert (chan); |