[v2,3/3] scandir: fix wrong assumption about errno [BZ #17804]
Commit Message
* Aurelien Jarno:
> 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.
I think it would be clearer if the set-errno-to-zero logic would
happen directly before the READDIR call, where it is needed.
Something like this (untested):
@@ -37,8 +37,11 @@ SCANDIR_TAIL (DIR *dp,
if (dp == NULL)
return -1;
+ /* errno is set to zero before the call to READDIR. The original
+ errno value is restored in case of success, so that the caller
+ does not observe that the zero errno value, which is not
+ permitted by POSIX. */
int save = errno;
- __set_errno (0);
int result;
struct scandir_cancel_struct c = { .dp = dp };
@@ -47,22 +50,41 @@ SCANDIR_TAIL (DIR *dp,
DIRENT_TYPE **v = NULL;
size_t vsize = 0;
DIRENT_TYPE *d;
- while ((d = READDIR (dp)) != NULL)
+ while (true)
{
+ __set_errno (0);
+ DIRENT_TYPE *d = READDIR (dp);
+
+ /* Check if reading the directory is complete. */
+ if (d == NULL)
+ {
+ if (errno == 0)
+ {
+ /* The entire directory was read successfully. */
+ __closedir (dp);
+
+ /* Sort the list if we have a comparison function to
+ sort with. */
+ if (cmp != NULL)
+ qsort (v, c.cnt, sizeof *v, (__compar_fn_t) cmp);
+
+ *namelist = v;
+ result = c.cnt;
+ }
+ else /* errno != 0 */
+ {
+ __scandir_cancel_handler (&c);
+ result = -1;
+ }
+ break;
+ } /* Directory reading completed. */
+
if (select != NULL)
{
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. */
- __set_errno (0);
-
if (!selected)
continue;
}
- else
- __set_errno (0);
if (__glibc_unlikely (c.cnt == vsize))
{
@@ -83,24 +105,6 @@ SCANDIR_TAIL (DIR *dp,
v[c.cnt++] = (DIRENT_TYPE *) memcpy (vnew, d, dsize);
}
- if (__glibc_likely (errno == 0))
- {
- __closedir (dp);
-
- /* Sort the list if we have a comparison function to sort with. */
- if (cmp != NULL)
- qsort (v, c.cnt, sizeof *v, (__compar_fn_t) cmp);
-
- *namelist = v;
- result = c.cnt;
- }
- else
- {
- /* This frees everything and calls closedir. */
- __scandir_cancel_handler (&c);
- result = -1;
- }
-
__libc_cleanup_pop (0);
if (result >= 0)