[PATCH] nss: Group merge does not react to ERANGE during merge (bug 33361)

[Collin Funk]

Florian Weimer <fweimer@redhat.com> writes:

> The break statement in CHECK_MERGE is expected to exit the surrounding
> while loop, not the do-while loop with in the macro.  Remove the
> do-while loop from the macro.  It is not needed to turn the macro
> expansion into a single statement due to the way CHECK_MERGE is used
> (and the statement expression would cover this anyway).
>
> Tested on x86_64-linux-gnu.  We gave a glibc build with this change to
> the user who encountered this, and it fixed their problem.  A colleague
> is also working on a proper glibc test case for this.
>
> Okay to push?
>
> Thanks,
> Florian
>
> ---
>  nss/getXXbyYY_r.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/nss/getXXbyYY_r.c b/nss/getXXbyYY_r.c
> index eae6c3480e..2b0735fb6a 100644
> --- a/nss/getXXbyYY_r.c
> +++ b/nss/getXXbyYY_r.c
> @@ -157,19 +157,15 @@ __merge_einval (LOOKUP_TYPE *a,
>  
>  #define CHECK_MERGE(err, status)		\
>    ({						\
> -    do						\
> +    if (err)					\
>        {						\
> -	if (err)				\
> -	  {					\
> -	    __set_errno (err);			\
> -	    if (err == ERANGE)			\
> -	      status = NSS_STATUS_TRYAGAIN;	\
> -	    else				\
> -	      status = NSS_STATUS_UNAVAIL;	\
> -	    break;				\
> -	  }					\
> +	__set_errno (err);			\
> +	if (err == ERANGE)			\
> +	  status = NSS_STATUS_TRYAGAIN;		\
> +	else					\
> +	  status = NSS_STATUS_UNAVAIL;		\
> +	break;					\
>        }						\
> -    while (0);					\
>    })
>  
>  /* Type of the lookup function we need here.  */
>
> base-commit: ed5da9d659783e02354a00c7f66b9e8da03fb584

The existing code seems unintentional, since the status would likely
(certainly?) be overwritten by another call to DL_CALL_FCT. Is that what
the user experienced?

If so:

Reviewed-by: Collin Funk <collin.funk1@gmail.com>

But definitely worth adding a test case.

Collin