[svn] gpgme - r1270 - trunk/gpgme
svn author marcus
cvs at cvs.gnupg.org
Tue Oct 2 14:02:19 CEST 2007
Author: marcus
Date: 2007-10-02 14:02:08 +0200 (Tue, 02 Oct 2007)
New Revision: 1270
Modified:
trunk/gpgme/ChangeLog
trunk/gpgme/engine-gpgsm.c
trunk/gpgme/priv-io.h
trunk/gpgme/w32-glib-io.c
trunk/gpgme/w32-qt-io.cpp
Log:
2007-10-02 Marcus Brinkmann <marcus at g10code.de>
* 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.
Modified: trunk/gpgme/ChangeLog
===================================================================
--- trunk/gpgme/ChangeLog 2007-09-28 17:30:11 UTC (rev 1269)
+++ trunk/gpgme/ChangeLog 2007-10-02 12:02:08 UTC (rev 1270)
@@ -1,3 +1,12 @@
+2007-10-02 Marcus Brinkmann <marcus at g10code.de>
+
+ * 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 <wk at g10code.com>
* engine-gpgsm.c (iocb_data_t): Add SERVER_FD_STR.
Modified: trunk/gpgme/engine-gpgsm.c
===================================================================
--- trunk/gpgme/engine-gpgsm.c 2007-09-28 17:30:11 UTC (rev 1269)
+++ trunk/gpgme/engine-gpgsm.c 2007-10-02 12:02:08 UTC (rev 1270)
@@ -54,7 +54,7 @@
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 @@
#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 @@
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)
Modified: trunk/gpgme/priv-io.h
===================================================================
--- trunk/gpgme/priv-io.h 2007-09-28 17:30:11 UTC (rev 1269)
+++ trunk/gpgme/priv-io.h 2007-10-02 12:02:08 UTC (rev 1270)
@@ -64,7 +64,13 @@
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 */
Modified: trunk/gpgme/w32-glib-io.c
===================================================================
--- trunk/gpgme/w32-glib-io.c 2007-09-28 17:30:11 UTC (rev 1269)
+++ trunk/gpgme/w32-glib-io.c 2007-10-02 12:02:08 UTC (rev 1270)
@@ -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 @@
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 @@
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);
}
Modified: trunk/gpgme/w32-qt-io.cpp
===================================================================
--- trunk/gpgme/w32-qt-io.cpp 2007-09-28 17:30:11 UTC (rev 1269)
+++ trunk/gpgme/w32-qt-io.cpp 2007-10-02 12:02:08 UTC (rev 1270)
@@ -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,13 +583,8 @@
{
fds[i].signaled = 0;
}
- else if (fds[i].frozen)
+ else if (fds[i].for_read)
{
- TRACE_ADD1 (dbg_help, "f0x%x ", fds[i].fd);
- fds[i].signaled = 0;
- }
- else if (fds[i].for_read )
- {
const QIODevice * const chan = find_channel (fds[i].fd, 0);
assert (chan);
fds[i].signaled = chan->bytesAvailable() > 0 ? 1 : 0 ;
More information about the Gnupg-commits
mailing list