From 52d8ed8dfb91a2b302c20bee27e9a28b7cb6a518 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Fri, 7 Jun 2019 11:17:53 +0200 Subject: [PATCH] core: Replace the posix close notify mechanism by a new generic one. * src/fdtable.c, src/fdtable.h: New. * src/posix-io.c (notify_table_item_s): Remove. (notify_table, notify_table_size, notify_table_lock): Remove. (_gpgme_io_pipe): Put new fds into the table. (_gpgme_io_dup): Ditto. (_gpgme_io_close): Replace notify stuff by a call to the fdtable. (_gpgme_io_set_close_notify): Remove. Change all callers to to use _gpgme_fdtable_add_close_notify. * src/Makefile.am (main_sources): Add new files. -- This is the first part or a larger change to unify the tracking of file descriptors. Right now this has only been implemented for Posix and thus the code will not yet build for Windows. Signed-off-by: Werner Koch --- src/Makefile.am | 1 + src/engine-assuan.c | 7 +- src/engine-backend.h | 1 + src/engine-g13.c | 7 +- src/engine-gpg.c | 28 +++--- src/engine-gpgsm.c | 35 +++++--- src/engine-spawn.c | 9 +- src/engine-uiserver.c | 12 +-- src/fdtable.c | 205 ++++++++++++++++++++++++++++++++++++++++++ src/fdtable.h | 43 +++++++++ src/posix-io.c | 153 ++++++++++--------------------- src/priv-io.h | 3 - src/w32-io.c | 2 +- 13 files changed, 358 insertions(+), 148 deletions(-) create mode 100644 src/fdtable.c create mode 100644 src/fdtable.h diff --git a/src/Makefile.am b/src/Makefile.am index d85a85c9..79028a17 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -74,6 +74,7 @@ main_sources = \ data-estream.c \ data-compat.c data-identify.c \ signers.c sig-notation.c \ + fdtable.c fdtable.h \ wait.c wait-global.c wait-private.c wait-user.c wait.h \ op-support.c \ encrypt.c encrypt-sign.c decrypt.c decrypt-verify.c verify.c \ diff --git a/src/engine-assuan.c b/src/engine-assuan.c index 497397db..0cca9e59 100644 --- a/src/engine-assuan.c +++ b/src/engine-assuan.c @@ -145,7 +145,7 @@ llass_get_req_version (void) } -static void +static gpg_error_t close_notify_handler (int fd, void *opaque) { engine_llass_t llass = opaque; @@ -158,6 +158,7 @@ close_notify_handler (int fd, void *opaque) llass->status_cb.fd = -1; llass->status_cb.tag = NULL; } + return 0; } @@ -720,8 +721,8 @@ start (engine_llass_t llass, const char *command) if (llass->status_cb.fd < 0) return gpg_error_from_syserror (); - if (_gpgme_io_set_close_notify (llass->status_cb.fd, - close_notify_handler, llass)) + if (_gpgme_fdtable_add_close_notify (llass->status_cb.fd, + close_notify_handler, llass)) { _gpgme_io_close (llass->status_cb.fd); llass->status_cb.fd = -1; diff --git a/src/engine-backend.h b/src/engine-backend.h index 4f33da1c..9bf54bc1 100644 --- a/src/engine-backend.h +++ b/src/engine-backend.h @@ -21,6 +21,7 @@ #define ENGINE_BACKEND_H #include "engine.h" +#include "fdtable.h" struct engine_ops { diff --git a/src/engine-g13.c b/src/engine-g13.c index 19dd8f47..7d448595 100644 --- a/src/engine-g13.c +++ b/src/engine-g13.c @@ -111,7 +111,7 @@ g13_get_req_version (void) } -static void +static gpg_error_t close_notify_handler (int fd, void *opaque) { engine_g13_t g13 = opaque; @@ -124,6 +124,7 @@ close_notify_handler (int fd, void *opaque) g13->status_cb.fd = -1; g13->status_cb.tag = NULL; } + return 0; } @@ -686,8 +687,8 @@ start (engine_g13_t g13, const char *command) if (g13->status_cb.fd < 0) return gpg_error_from_syserror (); - if (_gpgme_io_set_close_notify (g13->status_cb.fd, - close_notify_handler, g13)) + if (_gpgme_fdtable_add_close_notify (g13->status_cb.fd, + close_notify_handler, g13)) { _gpgme_io_close (g13->status_cb.fd); g13->status_cb.fd = -1; diff --git a/src/engine-gpg.c b/src/engine-gpg.c index dc2d9455..cce80d8d 100644 --- a/src/engine-gpg.c +++ b/src/engine-gpg.c @@ -173,7 +173,7 @@ gpg_io_event (void *engine, gpgme_event_io_t type, void *type_data) } -static void +static gpg_error_t close_notify_handler (int fd, void *opaque) { engine_gpg_t gpg = opaque; @@ -217,6 +217,7 @@ close_notify_handler (int fd, void *opaque) } } } + return 0; } /* If FRONT is true, push at the front of the list. Use this for @@ -525,10 +526,10 @@ gpg_new (void **engine, const char *file_name, const char *home_dir, rc = gpg_error_from_syserror (); goto leave; } - if (_gpgme_io_set_close_notify (gpg->status.fd[0], - close_notify_handler, gpg) - || _gpgme_io_set_close_notify (gpg->status.fd[1], - close_notify_handler, gpg)) + if (_gpgme_fdtable_add_close_notify (gpg->status.fd[0], + close_notify_handler, gpg) + || _gpgme_fdtable_add_close_notify (gpg->status.fd[1], + close_notify_handler, gpg)) { rc = gpg_error (GPG_ERR_GENERAL); goto leave; @@ -778,9 +779,10 @@ gpg_set_colon_line_handler (void *engine, engine_colon_line_handler_t fnc, gpg->colon.buffer = NULL; return saved_err; } - if (_gpgme_io_set_close_notify (gpg->colon.fd[0], close_notify_handler, gpg) - || _gpgme_io_set_close_notify (gpg->colon.fd[1], - close_notify_handler, gpg)) + if (_gpgme_fdtable_add_close_notify (gpg->colon.fd[0], + close_notify_handler, gpg) + || _gpgme_fdtable_add_close_notify (gpg->colon.fd[1], + close_notify_handler, gpg)) return gpg_error (GPG_ERR_GENERAL); gpg->colon.eof = 0; gpg->colon.fnc = fnc; @@ -1112,11 +1114,11 @@ build_argv (engine_gpg_t gpg, const char *pgmname) free_argv (argv); return saved_err; } - if (_gpgme_io_set_close_notify (fds[0], - close_notify_handler, gpg) - || _gpgme_io_set_close_notify (fds[1], - close_notify_handler, - gpg)) + if (_gpgme_fdtable_add_close_notify (fds[0], + close_notify_handler, gpg) + || _gpgme_fdtable_add_close_notify (fds[1], + close_notify_handler, + gpg)) { /* We leak fd_data_map and the fds. This is not easy to avoid and given that we reach this here only diff --git a/src/engine-gpgsm.c b/src/engine-gpgsm.c index ae5d8ef1..295703a7 100644 --- a/src/engine-gpgsm.c +++ b/src/engine-gpgsm.c @@ -138,11 +138,13 @@ gpgsm_get_req_version (void) } -static void +static gpg_error_t close_notify_handler (int fd, void *opaque) { engine_gpgsm_t gpgsm = opaque; + TRACE_BEG (DEBUG_SYSIO, "gpgsm:close_notify_handler", NULL, + "fd=%d", fd); assert (fd != -1); if (gpgsm->status_cb.fd == fd) { @@ -156,6 +158,7 @@ close_notify_handler (int fd, void *opaque) * The status fd however is closed right after it received the * "OK" from the command. So we use this event to also close * the diag fd. */ + TRACE_LOG ("closing diag fd"); _gpgme_io_close (gpgsm->diag_cb.fd); } else if (gpgsm->input_cb.fd == fd) @@ -174,6 +177,7 @@ close_notify_handler (int fd, void *opaque) free (gpgsm->input_helper_memory); gpgsm->input_helper_memory = NULL; } + TRACE_LOG ("ready with input_fd"); } else if (gpgsm->output_cb.fd == fd) { @@ -181,6 +185,7 @@ close_notify_handler (int fd, void *opaque) (*gpgsm->io_cbs.remove) (gpgsm->output_cb.tag); gpgsm->output_cb.fd = -1; gpgsm->output_cb.tag = NULL; + TRACE_LOG ("ready with output_fd"); } else if (gpgsm->message_cb.fd == fd) { @@ -188,6 +193,7 @@ close_notify_handler (int fd, void *opaque) (*gpgsm->io_cbs.remove) (gpgsm->message_cb.tag); gpgsm->message_cb.fd = -1; gpgsm->message_cb.tag = NULL; + TRACE_LOG ("ready with message_fd"); } else if (gpgsm->diag_cb.fd == fd) { @@ -195,7 +201,10 @@ close_notify_handler (int fd, void *opaque) (*gpgsm->io_cbs.remove) (gpgsm->diag_cb.tag); gpgsm->diag_cb.fd = -1; gpgsm->diag_cb.tag = NULL; + TRACE_LOG ("ready with diag_fd"); } + TRACE_SUC (""); + return 0; } @@ -544,19 +553,19 @@ gpgsm_new (void **engine, const char *file_name, const char *home_dir, #if !USE_DESCRIPTOR_PASSING if (!err - && (_gpgme_io_set_close_notify (gpgsm->input_cb.fd, - close_notify_handler, gpgsm) - || _gpgme_io_set_close_notify (gpgsm->output_cb.fd, - close_notify_handler, gpgsm) - || _gpgme_io_set_close_notify (gpgsm->message_cb.fd, - close_notify_handler, gpgsm))) + && (_gpgme_fdtable_add_close_notify (gpgsm->input_cb.fd, + close_notify_handler, gpgsm) + || _gpgme_fdtable_add_close_notify (gpgsm->output_cb.fd, + close_notify_handler, gpgsm) + || _gpgme_fdtable_add_close_notify (gpgsm->message_cb.fd, + close_notify_handler, gpgsm))) { err = gpg_error (GPG_ERR_GENERAL); goto leave; } #endif - if (!err && _gpgme_io_set_close_notify (gpgsm->diag_cb.fd, - close_notify_handler, gpgsm)) + if (!err && _gpgme_fdtable_add_close_notify (gpgsm->diag_cb.fd, + close_notify_handler, gpgsm)) { err = gpg_error (GPG_ERR_GENERAL); goto leave; @@ -816,8 +825,8 @@ gpgsm_set_fd (engine_gpgsm_t gpgsm, fd_type_t fd_type, const char *opt) iocb_data->fd = dir ? fds[0] : fds[1]; iocb_data->server_fd = dir ? fds[1] : fds[0]; - if (_gpgme_io_set_close_notify (iocb_data->fd, - close_notify_handler, gpgsm)) + if (_gpgme_fdtable_add_close_notify (iocb_data->fd, + close_notify_handler, gpgsm)) { err = gpg_error (GPG_ERR_GENERAL); goto leave_set_fd; @@ -1188,8 +1197,8 @@ start (engine_gpgsm_t gpgsm, const char *command) if (gpgsm->status_cb.fd < 0) return gpg_error_from_syserror (); - if (_gpgme_io_set_close_notify (gpgsm->status_cb.fd, - close_notify_handler, gpgsm)) + if (_gpgme_fdtable_add_close_notify (gpgsm->status_cb.fd, + close_notify_handler, gpgsm)) { _gpgme_io_close (gpgsm->status_cb.fd); gpgsm->status_cb.fd = -1; diff --git a/src/engine-spawn.c b/src/engine-spawn.c index 296d7f25..39428f29 100644 --- a/src/engine-spawn.c +++ b/src/engine-spawn.c @@ -84,7 +84,7 @@ static gpgme_error_t engspawn_cancel (void *engine); -static void +static gpg_error_t close_notify_handler (int fd, void *opaque) { engine_spawn_t esp = opaque; @@ -110,6 +110,7 @@ close_notify_handler (int fd, void *opaque) } } } + return 0; } @@ -180,8 +181,10 @@ build_fd_data_map (engine_spawn_t esp) esp->fd_data_map = NULL; return gpg_error_from_syserror (); } - if (_gpgme_io_set_close_notify (fds[0], close_notify_handler, esp) - || _gpgme_io_set_close_notify (fds[1], close_notify_handler, esp)) + if (_gpgme_fdtable_add_close_notify (fds[0], + close_notify_handler, esp) + || _gpgme_fdtable_add_close_notify (fds[1], + close_notify_handler, esp)) { /* FIXME: Need error cleanup. */ return gpg_error (GPG_ERR_GENERAL); diff --git a/src/engine-uiserver.c b/src/engine-uiserver.c index cb8e155d..e7896bbe 100644 --- a/src/engine-uiserver.c +++ b/src/engine-uiserver.c @@ -135,7 +135,7 @@ uiserver_get_req_version (void) } -static void +static gpg_error_t close_notify_handler (int fd, void *opaque) { engine_uiserver_t uiserver = opaque; @@ -179,6 +179,8 @@ close_notify_handler (int fd, void *opaque) uiserver->message_cb.fd = -1; uiserver->message_cb.tag = NULL; } + + return 0; } @@ -585,8 +587,8 @@ uiserver_set_fd (engine_uiserver_t uiserver, fd_type_t fd_type, const char *opt) iocb_data->fd = dir ? fds[0] : fds[1]; iocb_data->server_fd = dir ? fds[1] : fds[0]; - if (_gpgme_io_set_close_notify (iocb_data->fd, - close_notify_handler, uiserver)) + if (_gpgme_fdtable_add_close_notify (iocb_data->fd, + close_notify_handler, uiserver)) { err = gpg_error (GPG_ERR_GENERAL); goto leave_set_fd; @@ -921,8 +923,8 @@ start (engine_uiserver_t uiserver, const char *command) if (uiserver->status_cb.fd < 0) return gpg_error_from_syserror (); - if (_gpgme_io_set_close_notify (uiserver->status_cb.fd, - close_notify_handler, uiserver)) + if (_gpgme_fdtable_add_close_notify (uiserver->status_cb.fd, + close_notify_handler, uiserver)) { _gpgme_io_close (uiserver->status_cb.fd); uiserver->status_cb.fd = -1; diff --git a/src/fdtable.c b/src/fdtable.c new file mode 100644 index 00000000..2e682dea --- /dev/null +++ b/src/fdtable.c @@ -0,0 +1,205 @@ +/* fdtable.c - Keep track of file descriptors. + * Copyright (C) 2019 g10 Code GmbH + * + * This file is part of GPGME. + * + * GPGME is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * GPGME is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, see . + * SPDX-License-Identifier: LGPL-2.1-or-later + */ + +#if HAVE_CONFIG_H +#include +#endif +#include +#include + +#include "gpgme.h" +#include "debug.h" +#include "context.h" +#include "fdtable.h" + + +/* The table to hold information about file descriptors. Currently we + * use a linear search and extend the table as needed. Eventually we + * may swicth to a hash table and allocate items on the fly. */ +struct fdtable_item_s +{ + int fd; /* -1 indicates an unused entry. */ + + /* The callback to be called before the descriptor is actually closed. */ + struct { + fdtable_handler_t handler; + void *value; + } close_notify; +}; +typedef struct fdtable_item_s *fdtable_item_t; + +/* The actual table, its size and the lock to guard access. */ +static fdtable_item_t fdtable; +static unsigned int fdtablesize; +DEFINE_STATIC_LOCK (fdtable_lock); + + + +/* Insert FD into our file descriptor table. This function checks + * that FD is not yet in the table. On success 0 is returned; if FD + * is already in the table GPG_ERR_DUP_KEY is returned. Other error + * codes may also be returned. */ +gpg_error_t +_gpgme_fdtable_insert (int fd) +{ + gpg_error_t err; + int firstunused, idx; + + TRACE_BEG (DEBUG_SYSIO, __func__, NULL, "fd=%d", fd); + + if (fd < 0 ) + return TRACE_ERR (gpg_error (GPG_ERR_INV_ARG)); + + LOCK (fdtable_lock); + + firstunused = -1; + for (idx=0; idx < fdtablesize; idx++) + if (fdtable[idx].fd == -1) + { + if (firstunused == -1) + firstunused = idx; + } + else if (fdtable[idx].fd == fd) + { + err = gpg_error (GPG_ERR_DUP_KEY); + goto leave; + } + + if (firstunused == -1) + { + /* We need to increase the size of the table. The approach we + * take is straightforward to minimize the risk of bugs. */ + fdtable_item_t newtbl; + size_t newsize = fdtablesize + 64; + + newtbl = calloc (newsize, sizeof *newtbl); + if (!newtbl) + { + err = gpg_error_from_syserror (); + goto leave; + } + for (idx=0; idx < fdtablesize; idx++) + newtbl[idx] = fdtable[idx]; + for (; idx < newsize; idx++) + newtbl[idx].fd = -1; + + free (fdtable); + fdtable = newtbl; + idx = fdtablesize; + fdtablesize = newsize; + } + else + idx = firstunused; + + fdtable[idx].fd = fd; + fdtable[idx].close_notify.handler = NULL; + fdtable[idx].close_notify.value = NULL; + err = 0; + + leave: + UNLOCK (fdtable_lock); + return TRACE_ERR (err); +} + + +/* Add the close notification HANDLER to the table under the key FD. + * FD must exist. VALUE is a pointer passed to the handler along with + * the FD. */ +gpg_error_t +_gpgme_fdtable_add_close_notify (int fd, + fdtable_handler_t handler, void *value) +{ + gpg_error_t err; + int idx; + + TRACE_BEG (DEBUG_SYSIO, __func__, NULL, "fd=%d", fd); + + if (fd < 0 ) + return TRACE_ERR (gpg_error (GPG_ERR_INV_ARG)); + + LOCK (fdtable_lock); + + for (idx=0; idx < fdtablesize; idx++) + if (fdtable[idx].fd == fd) + break; + if (idx == fdtablesize) + { + err = gpg_error (GPG_ERR_NO_KEY); + goto leave; + } + + if (fdtable[idx].close_notify.handler) + { + err = gpg_error (GPG_ERR_DUP_VALUE); + goto leave; + } + + fdtable[idx].close_notify.handler = handler; + fdtable[idx].close_notify.value = value; + err = 0; + + leave: + UNLOCK (fdtable_lock); + return TRACE_ERR (err); +} + + +/* Remove FD from the table after calling the close handler. Note + * that at the time the close handler is called the FD has been + * removed form the table. Thus the close handler may not access the + * fdtable anymore and assume that FD is still there. Callers may + * want to handle the error code GPG_ERR_NO_KEY which indicates that + * FD is not anymore or not yet in the table. */ +gpg_error_t +_gpgme_fdtable_remove (int fd) +{ + gpg_error_t err; + int idx; + fdtable_handler_t handler; + void *handlervalue; + + TRACE_BEG (DEBUG_SYSIO, __func__, NULL, "fd=%d", fd); + + if (fd < 0 ) + return TRACE_ERR (gpg_error (GPG_ERR_INV_ARG)); + + LOCK (fdtable_lock); + + for (idx=0; idx < fdtablesize; idx++) + if (fdtable[idx].fd == fd) + break; + if (idx == fdtablesize) + { + UNLOCK (fdtable_lock); + return TRACE_ERR (gpg_error (GPG_ERR_NO_KEY)); + } + + handler = fdtable[idx].close_notify.handler; + fdtable[idx].close_notify.handler = NULL; + handlervalue = fdtable[idx].close_notify.value; + fdtable[idx].close_notify.value = NULL; + fdtable[idx].fd = -1; + + UNLOCK (fdtable_lock); + + err = handler? handler (fd, handlervalue) : 0; + + return TRACE_ERR (err); +} diff --git a/src/fdtable.h b/src/fdtable.h new file mode 100644 index 00000000..6038b249 --- /dev/null +++ b/src/fdtable.h @@ -0,0 +1,43 @@ +/* fdtable.h - Keep track of file descriptors. + * Copyright (C) 2019 g10 Code GmbH + * + * This file is part of GPGME. + * + * GPGME is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * GPGME is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, see . + * SPDX-License-Identifier: LGPL-2.1-or-later + */ + +#ifndef GPGME_FDTABLE_H +#define GPGME_FDTABLE_H + +/* The handler type associated with an FD. It is called with the FD + * and the registered pointer. The handler may return an error code + * but there is no guarantee that this code is used; in particular + * errors from close notifications can't inhibit the the closing. */ +typedef gpg_error_t (*fdtable_handler_t) (int, void*); + + +/* Insert a new FD into the table. */ +gpg_error_t _gpgme_fdtable_insert (int fd); + +/* Add a close notification handler to the FD item. */ +gpg_error_t _gpgme_fdtable_add_close_notify (int fd, + fdtable_handler_t handler, + void *value); + +/* Remove FD from the table. This also runs the close handlers. */ +gpg_error_t _gpgme_fdtable_remove (int fd); + + +#endif /*GPGME_FDTABLE_H*/ diff --git a/src/posix-io.c b/src/posix-io.c index e712ef28..b754ac34 100644 --- a/src/posix-io.c +++ b/src/posix-io.c @@ -59,6 +59,7 @@ #include "priv-io.h" #include "sema.h" #include "ath.h" +#include "fdtable.h" #include "debug.h" @@ -152,22 +153,6 @@ _gpgme_io_fd2str (char *buf, int buflen, int fd) return snprintf (buf, buflen, "%d", fd); } - -/* The table to hold notification handlers. We use a linear search - and extend the table as needed. */ -struct notify_table_item_s -{ - int fd; /* -1 indicates an unused entry. */ - _gpgme_close_notify_handler_t handler; - void *value; -}; -typedef struct notify_table_item_s *notify_table_item_t; - -static notify_table_item_t notify_table; -static size_t notify_table_size; -DEFINE_STATIC_LOCK (notify_table_lock); - - int _gpgme_io_read (int fd, void *buffer, size_t count) @@ -208,27 +193,43 @@ _gpgme_io_write (int fd, const void *buffer, size_t count) int _gpgme_io_pipe (int filedes[2], int inherit_idx) { + gpg_error_t err; + int res; int saved_errno; - int err; + int i; TRACE_BEG (DEBUG_SYSIO, "_gpgme_io_pipe", NULL, "inherit_idx=%i (GPGME uses it for %s)", inherit_idx, inherit_idx ? "reading" : "writing"); - err = pipe (filedes); - if (err < 0) - return TRACE_SYSRES (err); + res = pipe (filedes); + if (res < 0) + return TRACE_SYSRES (res); /* FIXME: Should get the old flags first. */ - err = fcntl (filedes[1 - inherit_idx], F_SETFD, FD_CLOEXEC); + res = fcntl (filedes[1 - inherit_idx], F_SETFD, FD_CLOEXEC); saved_errno = errno; - if (err < 0) + if (res < 0) { close (filedes[0]); close (filedes[1]); } errno = saved_errno; - if (err) - return TRACE_SYSRES (err); + if (res) + return TRACE_SYSRES (res); + + for (i=0; i < 2; i++) + { + err = _gpgme_fdtable_insert (filedes[i]); + if (err) + { + TRACE_LOG ("fdtable_insert failed for fd=%d: %s\n", + filedes[i], gpg_strerror (err)); + close (filedes[0]); + close (filedes[1]); + gpg_err_set_errno (EIO); + return TRACE_SYSRES (-1); + } + } TRACE_SUC ("read fd=%d write fd=%d", filedes[0], filedes[1]); return 0; @@ -238,38 +239,23 @@ _gpgme_io_pipe (int filedes[2], int inherit_idx) int _gpgme_io_close (int fd) { + gpg_error_t err; int res; - _gpgme_close_notify_handler_t handler = NULL; - void *handler_value; - int idx; TRACE_BEG (DEBUG_SYSIO, "_gpgme_io_close", NULL, "fd=%d", fd); - if (fd == -1) - { - errno = EINVAL; - return TRACE_SYSRES (-1); - } + return TRACE_SYSRES (0); /* Igore invalid FDs. */ - /* First call the notify handler. */ - LOCK (notify_table_lock); - for (idx=0; idx < notify_table_size; idx++) + /* First remove from the table which also runs the close handlers. + * Having the FD not (yet) in the table is possible and thus we + * ignore that error code. */ + err = _gpgme_fdtable_remove (fd); + if (err && gpg_err_code (err) != GPG_ERR_NO_KEY) { - if (notify_table[idx].fd == fd) - { - handler = notify_table[idx].handler; - handler_value = notify_table[idx].value; - notify_table[idx].handler = NULL; - notify_table[idx].value = NULL; - notify_table[idx].fd = -1; /* Mark slot as free. */ - break; - } - } - UNLOCK (notify_table_lock); - if (handler) - { - TRACE_LOG ("invoking close handler %p/%p", handler, handler_value); - handler (fd, handler_value); + TRACE_LOG ("fdtable_remove failed for fd=%d: %s\n", + fd, gpg_strerror (err)); + gpg_err_set_errno (EINVAL); + return TRACE_SYSRES (-1); } /* Then do the close. */ @@ -278,59 +264,6 @@ _gpgme_io_close (int fd) } -int -_gpgme_io_set_close_notify (int fd, _gpgme_close_notify_handler_t handler, - void *value) -{ - int res = 0; - int idx; - - TRACE_BEG (DEBUG_SYSIO, "_gpgme_io_set_close_notify", NULL, - "fd=%d close_handler=%p/%p", fd, handler, value); - - assert (fd != -1); - - LOCK (notify_table_lock); - for (idx=0; idx < notify_table_size; idx++) - if (notify_table[idx].fd == -1) - break; - if (idx == notify_table_size) - { - /* We need to increase the size of the table. The approach we - take is straightforward to minimize the risk of bugs. */ - notify_table_item_t newtbl; - size_t newsize = notify_table_size + 64; - - newtbl = calloc (newsize, sizeof *newtbl); - if (!newtbl) - { - res = -1; - goto leave; - } - for (idx=0; idx < notify_table_size; idx++) - newtbl[idx] = notify_table[idx]; - for (; idx < newsize; idx++) - { - newtbl[idx].fd = -1; - newtbl[idx].handler = NULL; - newtbl[idx].value = NULL; - } - free (notify_table); - notify_table = newtbl; - idx = notify_table_size; - notify_table_size = newsize; - } - notify_table[idx].fd = fd; - notify_table[idx].handler = handler; - notify_table[idx].value = value; - - leave: - UNLOCK (notify_table_lock); - - return TRACE_SYSRES (res); -} - - int _gpgme_io_set_nonblocking (int fd) { @@ -885,17 +818,29 @@ _gpgme_io_sendmsg (int fd, const struct msghdr *msg, int flags) int _gpgme_io_dup (int fd) { + gpg_error_t err; int new_fd; do new_fd = dup (fd); while (new_fd == -1 && errno == EINTR); - TRACE (DEBUG_SYSIO, "_gpgme_io_dup", NULL, "fd=%d -> fd=%d", fd, new_fd); + TRACE (DEBUG_SYSIO, __func__, NULL, "fd=%d -> fd=%d", fd, new_fd); + err = _gpgme_fdtable_insert (new_fd); + if (err) + { + TRACE (DEBUG_SYSIO, __func__, NULL, + "fdtable_insert failed for fd=%d: %s\n", + new_fd, gpg_strerror (err)); + close (new_fd); + new_fd = -1; + gpg_err_set_errno (EIO); + } return new_fd; } + int _gpgme_io_socket (int domain, int type, int proto) diff --git a/src/priv-io.h b/src/priv-io.h index d21f1b34..f40cdffc 100644 --- a/src/priv-io.h +++ b/src/priv-io.h @@ -68,9 +68,6 @@ int _gpgme_io_read (int fd, void *buffer, size_t count); int _gpgme_io_write (int fd, const void *buffer, size_t count); int _gpgme_io_pipe (int filedes[2], int inherit_idx); int _gpgme_io_close (int fd); -typedef void (*_gpgme_close_notify_handler_t) (int,void*); -int _gpgme_io_set_close_notify (int fd, _gpgme_close_notify_handler_t handler, - void *value); int _gpgme_io_set_nonblocking (int fd); /* Under Windows do not allocate a console. */ diff --git a/src/w32-io.c b/src/w32-io.c index c5c21f59..ea9622e9 100644 --- a/src/w32-io.c +++ b/src/w32-io.c @@ -143,7 +143,7 @@ static struct /* The context of an associated writer object or NULL. */ struct writer_context_s *writer; - /* A notification handler. Noet that we current support only one + /* A notification handler. Note that we current support only one * callback per fd. */ struct { _gpgme_close_notify_handler_t handler;