* 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-10-02 Marcus Brinkmann <marcus@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@g10code.com>
* engine-gpgsm.c (iocb_data_t): Add SERVER_FD_STR.
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;
#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);
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)
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 */
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];
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
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);
}
-/* 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
{
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);