[pacman-dev] [PATCH] make alpm_unlock and trans_interrupt async safe
RET_ERR calls _alpm_log which includes calls that are not safe for use in asynchronous signal handlers (see signal(7)). Replace it in functions called from our signal handlers with a new macro RET_ERR_ASYNC which is identical except that it lacks the call to _alpm_log. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/handle.c | 2 +- lib/libalpm/trans.c | 8 +++++--- lib/libalpm/util.h | 4 ++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index e4bde1a..578647e 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -141,7 +141,7 @@ int SYMEXPORT alpm_unlock(alpm_handle_t *handle) handle->lockfd = -1; if(unlink(handle->lockfile) != 0) { - RET_ERR(handle, ALPM_ERR_SYSTEM, -1); + RET_ERR_ASYNC(handle, ALPM_ERR_SYSTEM, -1); } else { return 0; } diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index 239d6a1..30b288a 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -233,7 +233,9 @@ int SYMEXPORT alpm_trans_commit(alpm_handle_t *handle, alpm_list_t **data) return 0; } -/** Interrupt a transaction. */ +/** Interrupt a transaction. + * @note Safe to call from inside signal handlers. + */ int SYMEXPORT alpm_trans_interrupt(alpm_handle_t *handle) { alpm_trans_t *trans; @@ -242,9 +244,9 @@ int SYMEXPORT alpm_trans_interrupt(alpm_handle_t *handle) CHECK_HANDLE(handle, return -1); trans = handle->trans; - ASSERT(trans != NULL, RET_ERR(handle, ALPM_ERR_TRANS_NULL, -1)); + ASSERT(trans != NULL, RET_ERR_ASYNC(handle, ALPM_ERR_TRANS_NULL, -1)); ASSERT(trans->state == STATE_COMMITING || trans->state == STATE_INTERRUPTED, - RET_ERR(handle, ALPM_ERR_TRANS_TYPE, -1)); + RET_ERR_ASYNC(handle, ALPM_ERR_TRANS_TYPE, -1)); trans->state = STATE_INTERRUPTED; diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 9817939..f47c26e 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -71,6 +71,10 @@ void _alpm_alloc_fail(size_t size); (handle)->pm_errno = (err); \ return (ret); } while(0) +#define RET_ERR_ASYNC(handle, err, ret) do { \ + (handle)->pm_errno = (err); \ + return (ret); } while(0) + #define DOUBLE_EQ(x, y) (fabs((x) - (y)) < DBL_EPSILON) #define CHECK_HANDLE(handle, action) do { if(!(handle)) { action; } (handle)->pm_errno = 0; } while(0) -- 2.7.1
On Mon, Feb 22, 2016 at 02:13:59PM -0500, Andrew Gregory wrote:
RET_ERR calls _alpm_log which includes calls that are not safe for use in asynchronous signal handlers (see signal(7)). Replace it in functions called from our signal handlers with a new macro RET_ERR_ASYNC which is identical except that it lacks the call to _alpm_log.
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
I haven't actually looked at the patch, but I think RET_ERR_ASYNC is a terrible name. Maybe RET_ERR_NOLOG since that's the actual effect? If you want it more general, RET_ERR_ASYNC_SAFE?
lib/libalpm/handle.c | 2 +- lib/libalpm/trans.c | 8 +++++--- lib/libalpm/util.h | 4 ++++ 3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index e4bde1a..578647e 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -141,7 +141,7 @@ int SYMEXPORT alpm_unlock(alpm_handle_t *handle) handle->lockfd = -1;
if(unlink(handle->lockfile) != 0) { - RET_ERR(handle, ALPM_ERR_SYSTEM, -1); + RET_ERR_ASYNC(handle, ALPM_ERR_SYSTEM, -1); } else { return 0; } diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index 239d6a1..30b288a 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -233,7 +233,9 @@ int SYMEXPORT alpm_trans_commit(alpm_handle_t *handle, alpm_list_t **data) return 0; }
-/** Interrupt a transaction. */ +/** Interrupt a transaction. + * @note Safe to call from inside signal handlers. + */ int SYMEXPORT alpm_trans_interrupt(alpm_handle_t *handle) { alpm_trans_t *trans; @@ -242,9 +244,9 @@ int SYMEXPORT alpm_trans_interrupt(alpm_handle_t *handle) CHECK_HANDLE(handle, return -1);
trans = handle->trans; - ASSERT(trans != NULL, RET_ERR(handle, ALPM_ERR_TRANS_NULL, -1)); + ASSERT(trans != NULL, RET_ERR_ASYNC(handle, ALPM_ERR_TRANS_NULL, -1)); ASSERT(trans->state == STATE_COMMITING || trans->state == STATE_INTERRUPTED, - RET_ERR(handle, ALPM_ERR_TRANS_TYPE, -1)); + RET_ERR_ASYNC(handle, ALPM_ERR_TRANS_TYPE, -1));
trans->state = STATE_INTERRUPTED;
diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 9817939..f47c26e 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -71,6 +71,10 @@ void _alpm_alloc_fail(size_t size); (handle)->pm_errno = (err); \ return (ret); } while(0)
+#define RET_ERR_ASYNC(handle, err, ret) do { \ + (handle)->pm_errno = (err); \ + return (ret); } while(0) + #define DOUBLE_EQ(x, y) (fabs((x) - (y)) < DBL_EPSILON)
#define CHECK_HANDLE(handle, action) do { if(!(handle)) { action; } (handle)->pm_errno = 0; } while(0) -- 2.7.1
participants (2)
-
Andrew Gregory
-
Dave Reisner