[v4,02/13] support: Add <support/xdirent.h>

  Use static functions for readdir/readdir_r, so that
-D_FILE_OFFSET_BITS=64 does not improperly redirect calls to the wrong
implementation.
---
 support/Makefile                  |  6 +++
 support/support_readdir_check.c   | 29 +++++++++++++
 support/support_readdir_r_check.c | 29 +++++++++++++
 support/tst-xdirent.c             | 70 +++++++++++++++++++++++++++++++
 support/xclosedir.c               | 28 +++++++++++++
 support/xdirent.h                 | 53 +++++++++++++++++++++++
 support/xfdopendir.c              | 30 +++++++++++++
 support/xopendir.c                | 30 +++++++++++++
 8 files changed, 275 insertions(+)
 create mode 100644 support/support_readdir_check.c
 create mode 100644 support/support_readdir_r_check.c
 create mode 100644 support/tst-xdirent.c
 create mode 100644 support/xclosedir.c
 create mode 100644 support/xdirent.h
 create mode 100644 support/xfdopendir.c
 create mode 100644 support/xopendir.c
  

Florian Weimer <fweimer@redhat.com> writes:
> Use static functions for readdir/readdir_r, so that
> -D_FILE_OFFSET_BITS=64 does not improperly redirect calls to the wrong
> implementation.

LGTM but one question about the DIAG_* placement.

Reviewed-by: DJ Delorie <dj@redhat.com>

> diff --git a/support/Makefile b/support/Makefile
> +  support_readdir_check \
> +  support_readdir_r_check \
> +  xclosedir \
> +  xfdopendir \
> +  xopendir \

Ok.

> +  tst-xdirent \

Ok.

> diff --git a/support/support_readdir_check.c b/support/support_readdir_check.c
> +/* Error-checking helper for xreaddir, xreaddir64.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/xdirent.h>
> +
> +#include <support/check.h>
> +
> +void *
> +support_readdir_check (const char *name, void *result)
> +{
> +  if (result == NULL && errno != 0)
> +    FAIL_EXIT1 ("%s: %m", name);
> +  return result;
> +}

Ok.

> diff --git a/support/support_readdir_r_check.c b/support/support_readdir_r_check.c
> +/* Error-checking helper for xreaddir_r, xreaddir64_r.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/xdirent.h>
> +
> +#include <support/check.h>
> +
> +int
> +support_readdir_r_check (const char *name, int result)
> +{
> +  if (result != 0)
> +    FAIL_EXIT1 ("%s: %m", name);
> +  return result;
> +}

Ok.

> diff --git a/support/tst-xdirent.c b/support/tst-xdirent.c
> +/* Compile test for error-checking wrappers for <dirent.h>
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/xdirent.h>
> +
> +#include <libc-diag.h>
> +#include <support/check.h>
> +#include <unistd.h>

Ok.

> +static int
> +do_test (void)
> +{
> +  {
> +    DIR *d = xopendir (".");
> +    struct dirent *e = xreaddir (d);
> +    /* Assume that the "." special entry always comes first.  */
> +    TEST_COMPARE_STRING (e->d_name, ".");
> +    xclosedir (d);
> +  }
> +
> +  {
> +    DIR *d = xopendir (".");
> +    struct dirent64 *e = xreaddir64 (d);
> +    TEST_COMPARE_STRING (e->d_name, ".");
> +    xclosedir (d);
> +  }

Ok.

> +  /* The functions readdir_r, readdir64_r were deprecated in glibc 2.24.  */
> +  DIAG_PUSH_NEEDS_COMMENT;
> +  DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wdeprecated-declarations");

Does this need to go before the headers that define those?  I'm thinking
of the problem I had with FORTIFY_SOURCE and diagnostic warnings.

> +  {
> +    DIR *d = xopendir (".");
> +    struct dirent buf;
> +    struct dirent *e;
> +    TEST_COMPARE (xreaddir_r (d, &buf, &e), 0);
> +    TEST_COMPARE_STRING (e->d_name, ".");
> +    xclosedir (d);
> +  }

Ok.

> +  {
> +    DIR *d = xopendir (".");
> +    struct dirent64 buf;
> +    struct dirent64 *e;
> +    TEST_COMPARE (xreaddir64_r (d, &buf, &e), 0);
> +    TEST_COMPARE_STRING (e->d_name, ".");
> +    xclosedir (d);
> +  }

Ok.

> +  DIAG_POP_NEEDS_COMMENT;
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

Ok.

> diff --git a/support/xclosedir.c b/support/xclosedir.c
> +/* Error-checking wrapper for closedir.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/xdirent.h>
> +
> +#include <support/check.h>

Ok.

> +void
> +xclosedir (DIR *dir)
> +{
> +  if (closedir (dir) != 0)
> +    FAIL_EXIT1 ("closedir: %m");
> +}

Ok.

> diff --git a/support/xdirent.h b/support/xdirent.h
> +/* Error-checking wrappers for <dirent.h>
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef SUPPORT_XDIRENT_H
> +#define SUPPORT_XDIRENT_H
> +
> +#include <dirent.h>
> +#include <errno.h>
> +#include <libc-diag.h>

Ok.

> +__BEGIN_DECLS
> +
> +DIR *xopendir (const char *path);
> +DIR *xfdopendir (int fd);
> +void xclosedir (DIR *);
> +
> +void *support_readdir_check (const char *, void *);
> +#define xreaddir(d)                                                     \
> +  ((struct dirent *) support_readdir_check ("readdir",                  \
> +                                            (errno = 0, readdir ((d)))))
> +#define xreaddir64(d)                                                   \
> +  ((struct dirent64 *) support_readdir_check ("readdir64",              \
> +                                              (errno = 0, readdir64 ((d)))))

Ok.

> +int support_readdir_r_check (const char *, int);
> +
> +/* The functions readdir_r, readdir64_r were deprecated in glibc 2.24.  */
> +DIAG_PUSH_NEEDS_COMMENT;
> +DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wdeprecated-declarations");
> +#define xreaddir_r(d, e, r)                                             \
> +  (support_readdir_r_check ("readdir_r", readdir_r ((d), (e), (r))))
> +#define xreaddir64_r(d, e, r) \
> +  (support_readdir_r_check ("readdir64_r", readdir64_r ((d), (e), (r))))
> +DIAG_POP_NEEDS_COMMENT;
> +
> +__END_DECLS
> +
> +#endif /* SUPPORT_XDIRENT_H */

Ok.

> diff --git a/support/xfdopendir.c b/support/xfdopendir.c
> new file mode 100644
> index 0000000000..d881d28c73
> --- /dev/null
> +++ b/support/xfdopendir.c
> @@ -0,0 +1,30 @@
> +/* Error-checking wrapper for fdopendir.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/xdirent.h>
> +
> +#include <support/check.h>
> +
> +DIR *
> +xfdopendir (int fd)
> +{
> +  DIR *result = fdopendir (fd);
> +  if (result == NULL)
> +    FAIL_EXIT1 ("fdopendir (%d): %m", fd);
> +  return result;
> +}

Ok.

> diff --git a/support/xopendir.c b/support/xopendir.c
> new file mode 100644
> index 0000000000..e4aee07fee
> --- /dev/null
> +++ b/support/xopendir.c
> @@ -0,0 +1,30 @@
> +/* Error-checking wrapper for opendir.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/xdirent.h>
> +
> +#include <support/check.h>
> +
> +DIR *
> +xopendir (const char *path)
> +{
> +  DIR *result = opendir (path);
> +  if (result == NULL)
> +    FAIL_EXIT1 ("opendir (\"%s\"): %m", path);
> +  return result;
> +}

Ok.
  

* DJ Delorie:

> Florian Weimer <fweimer@redhat.com> writes:
>> Use static functions for readdir/readdir_r, so that
>> -D_FILE_OFFSET_BITS=64 does not improperly redirect calls to the wrong
>> implementation.
>
> LGTM but one question about the DIAG_* placement.
>
> Reviewed-by: DJ Delorie <dj@redhat.com>

Thanks!

>> +  /* The functions readdir_r, readdir64_r were deprecated in glibc 2.24.  */
>> +  DIAG_PUSH_NEEDS_COMMENT;
>> +  DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wdeprecated-declarations");
>
> Does this need to go before the headers that define those?  I'm thinking
> of the problem I had with FORTIFY_SOURCE and diagnostic warnings.

No, I think it has to go around the inline functions, and if things get
inlined, around the call site as well (but not sure about the second
part).

Florian