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 <[email protected]>
> ---

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

Reply via email to