Re: [PATCH] Linux: Move getdents64 to <dirent.h>

[Carlos O'Donell <carlos at redhat dot com>]

On 7/25/19 9:42 AM, Florian Weimer wrote:
> This matches the location of the declaration in musl.
>
> [Passed testing on x86_64-linux-gnu, i686-linux-gnu, and
> build-many-glibcs.py (with GCC 9).  I do think we should make this
> change before the release because it's going to be the first one with
> getdents64.]
>
> 2019-07-25  Florian Weimer  <fweimer@redhat.com>
>
> 	Linux: Move declaration of getdents64 to <dirent.h>.
> 	* bits/dirent_ext.h: New file.
> 	* dirent/Makefile (headers): Add bits/dirent_ext.h.
> 	* dirent/dirent.h: Include <bits/dirent_ext.h>.
> 	* sysdeps/unix/sysv/linux/bits/dirent_ext.h: New file.
> 	* sysdeps/unix/sysv/linux/bits/unistd_ext.h (getdents64): Remove
> 	declaration.
> 	* manual/filesys.texi (Low-level Directory Access): Update header
> 	to dirent.h.

Adhemerval already reviewed this, but I double checked it also because
we are trying to make sure we don't make any mistakes here.

Please commit this immediately for master so the release goes out with
the interface definition in the expected header. This avoids having to
update the linux man pages twice also when we later change the header
location. I think unistd.h is just wrong for this Linux interface.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> diff --git a/bits/dirent_ext.h b/bits/dirent_ext.h
> new file mode 100644
> index 0000000000..279fd25d25
> --- /dev/null
> +++ b/bits/dirent_ext.h
> @@ -0,0 +1,21 @@
> +/* System-specific extensions of <dirent.h>, generic version.

OK.

> +   Copyright (C) 2019 Free Software Foundation, Inc.

OK.

> +   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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _DIRENT_H
> +# error "Never include <bits/dirent_ext.h> directly; use <dirent.h> instead."
> +#endif

OK.

> diff --git a/dirent/Makefile b/dirent/Makefile
> index b002214c3d..144e448b3d 100644
> --- a/dirent/Makefile
> +++ b/dirent/Makefile
> @@ -22,7 +22,7 @@ subdir		:= dirent
>   
>   include ../Makeconfig
>   
> -headers		:= dirent.h bits/dirent.h
> +headers		:= dirent.h bits/dirent.h bits/dirent_ext.h

OK.

>   routines	:= opendir closedir readdir readdir_r rewinddir \
>   		   seekdir telldir scandir alphasort versionsort \
>   		   getdents getdents64 dirfd readdir64 readdir64_r scandir64 \
> diff --git a/dirent/dirent.h b/dirent/dirent.h
> index fc4d4859fe..d4650db17e 100644
> --- a/dirent/dirent.h
> +++ b/dirent/dirent.h
> @@ -401,4 +401,6 @@ extern int versionsort64 (const struct dirent64 **__e1,
>   
>   __END_DECLS
>   
> +#include <bits/dirent_ext.h>

OK. Include the *_ext.h header.

> +
>   #endif /* dirent.h  */
> diff --git a/manual/filesys.texi b/manual/filesys.texi
> index 513319418a..d31dbb24b4 100644
> --- a/manual/filesys.texi
> +++ b/manual/filesys.texi
> @@ -843,7 +843,7 @@ Directory data is obtained from a file descriptor, as created by the
>   @xref{Opening and Closing Files}.
>   
>   @deftypefun ssize_t getdents64 (int @var{fd}, void *@var{buffer}, size_t @var{length})
> -@standards{Linux, unistd.h}
> +@standards{Linux, dirent.h}

OK.

>   @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
>   The @code{getdents64} function reads at most @var{length} bytes of
>   directory entry data from the file descriptor @var{fd} and stores it
> diff --git a/sysdeps/unix/sysv/linux/bits/dirent_ext.h b/sysdeps/unix/sysv/linux/bits/dirent_ext.h
> new file mode 100644
> index 0000000000..bac8f95b8f
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/dirent_ext.h
> @@ -0,0 +1,33 @@
> +/* System-specific extensions of <dirent.h>.  Linux version.

OK.

> +   Copyright (C) 2019 Free Software Foundation, Inc.

OK.

> +   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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _DIRENT_H
> +# error "Never include <bits/dirent_ext.h> directly; use <dirent.h> instead."
> +#endif

OK.

> +
> +__BEGIN_DECLS
> +
> +#ifdef __USE_GNU
> +/* Read from the directory descriptor FD into LENGTH bytes at BUFFER.
> +   Return the number of bytes read on success (0 for end of
> +   directory), and -1 for failure.  */
> +extern __ssize_t getdents64 (int __fd, void *__buffer, size_t __length)
> +  __THROW __nonnull ((2));

OK.

> +#endif
> +
> +__END_DECLS
> diff --git a/sysdeps/unix/sysv/linux/bits/unistd_ext.h b/sysdeps/unix/sysv/linux/bits/unistd_ext.h
> index 6e7b2bb83d..0061172f23 100644
> --- a/sysdeps/unix/sysv/linux/bits/unistd_ext.h
> +++ b/sysdeps/unix/sysv/linux/bits/unistd_ext.h
> @@ -22,12 +22,6 @@
>   
>   #ifdef __USE_GNU
>   
> -/* Read from the directory descriptor FD into LENGTH bytes at BUFFER.
> -   Return the number of bytes read on success (0 for end of
> -   directory), and -1 for failure.  */
> -extern ssize_t getdents64 (int __fd, void *__buffer, size_t __length)
> -  __THROW __nonnull ((2));

OK.

> -
>   /* Return the kernel thread ID (TID) of the current thread.  The
>      returned value is not subject to caching.  Most Linux system calls
>      accept a TID in place of a PID.  Using the TID to change properties


--
Cheers,
Carlos.