Re: [PATCH v2 3/3] scandir: fix wrong assumption about errno [BZ #17804]

["Dmitry V. Levin" <ldv at altlinux dot org>]

On Fri, Dec 22, 2017 at 08:31:01PM +0100, Aurelien Jarno wrote:
> malloc and realloc may set errno to ENOMEM even if they are successful.
> The scandir code wrongly assume that they do not change errno, this
> causes scandir to fail with ENOMEM even if malloc succeed.
> 
> The code already handles that readdir might set errno by calling
> __set_errno (0) to clear the error. Move that part at the end of the
> loop to also take malloc and realloc into account.
> 
> Changelog:
> 	[BZ #17804]
> 	* dirent/scandir-tail.c (SCANDIR_TAIL): Move __set_errno (0) at the
> 	end of the loop. Improve comments.
> ---
>  ChangeLog             |  6 ++++++
>  dirent/scandir-tail.c | 13 ++++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 8368b3006d..713c5ade6d 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,9 @@
> +2017-12-22  Aurelien Jarno  <aurelien@aurel32.net>
> +
> +	[BZ #17804]
> +	* dirent/scandir-tail.c (SCANDIR_TAIL): Move __set_errno (0) at the
> +	end of the loop. Improve comments.
> +
>  2017-12-22  Zack Weinberg  <zackw@panix.com>
>  
>  	[BZ #22615]
> diff --git a/dirent/scandir-tail.c b/dirent/scandir-tail.c
> index 068c644c4e..927791fad0 100644
> --- a/dirent/scandir-tail.c
> +++ b/dirent/scandir-tail.c
> @@ -53,16 +53,14 @@ SCANDIR_TAIL (DIR *dp,
>          {
>            int selected = (*select) (d);
>  
> -	  /* The SELECT function might have changed errno.  It was
> -	     zero before and it need to be again to make the later
> -	     tests work.  */
> +	  /* The SELECT function might have set errno to non-zero on
> +	     success.  It was zero before and it need to be again to

s/it need to be/it needs to be/

> +	     make the later tests work.  */
>  	  __set_errno (0);
>  
>            if (!selected)
>              continue;
>          }
> -      else
> -        __set_errno (0);
>  
>        if (__glibc_unlikely (c.cnt == vsize))
>          {
> @@ -81,6 +79,11 @@ SCANDIR_TAIL (DIR *dp,
>        if (vnew == NULL)
>          break;
>        v[c.cnt++] = (DIRENT_TYPE *) memcpy (vnew, d, dsize);
> +
> +      /* Ignore errors from readdir, malloc or realloc.  These functions
> +	 might have set errno to non-zero on success.  It was zero before
> +	 and it need to be again to make the latter tests work.  */

Likewise.

> +      __set_errno (0);
>      }
>  
>    if (__glibc_likely (errno == 0))

Besides these two typos in comments, the patch is fine.


-- 
ldv

Attachment: signature.asc
Description: PGP signature