[PATCH] include/sys/stat.h: Add __USE_FILE_OFFSET64 redirection

[PATCH] include/sys/stat.h: Add __USE_FILE_OFFSET64 redirection

[Samuel Thibault]

Tests use include/sys/stat.h, which redirects the tests'
{,l,f}stat calls to __{,l,f}xstat calls. When a test is specifying
__USE_FILE_OFFSET64 (such as support/test-container.c), this
becomes bogus because while {,l,f}stat have the 64bit redirection,
__{,l,f}xstat did not have yet. This adds them.
---
 include/sys/stat.h | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/include/sys/stat.h b/include/sys/stat.h
index 108cb0c9bf..692850740f 100644
--- a/include/sys/stat.h
+++ b/include/sys/stat.h
@@ -29,14 +29,25 @@ in_blkcnt_t_range (__blkcnt64_t v)
 }
 
 /* Now define the internal interfaces. */
+#ifndef __USE_FILE_OFFSET64
 extern int __stat (const char *__file, struct stat *__buf);
-extern int __stat64 (const char *__file, struct stat64 *__buf);
 extern int __fstat (int __fd, struct stat *__buf);
-extern int __fstat64 (int __fd, struct stat64 *__buf);
 extern int __lstat (const char *__file, struct stat *__buf);
-extern int __lstat64 (const char *__file, struct stat64 *__buf);
 extern int __fstatat (int dirfd, const char *pathname, struct stat *buf,
 		      int flags);
+#else
+extern int __REDIRECT_NTH (__stat,
+		(const char *__file, struct stat *__buf), __stat64);
+extern int __REDIRECT_NTH (__fstat,
+		(int __fd, struct stat *__buf), __fstat64);
+extern int __REDIRECT_NTH (__lstat,
+		(const char *__file, struct stat *__buf), __lstat64);
+extern int __REDIRECT_NTH(__fstatat,
+		(int dirfd, const char *pathname, struct stat *buf, int flags), __fstatat64);
+#endif
+extern int __stat64 (const char *__file, struct stat64 *__buf);
+extern int __fstat64 (int __fd, struct stat64 *__buf);
+extern int __lstat64 (const char *__file, struct stat64 *__buf);
 extern int __fstatat64 (int dirfd, const char *pathname, struct stat64 *buf,
 			int flags);
 # if IS_IN (libc) || (IS_IN (rtld) && !defined NO_RTLD_HIDDEN)
@@ -82,12 +93,25 @@ extern int __xmknod (int __ver, const char *__path, __mode_t __mode,
 extern int __xmknodat (int __ver, int __fd, const char *__path,
 		       __mode_t __mode, __dev_t *__dev);
 
+#ifndef __USE_FILE_OFFSET64
 int __fxstat (int __ver, int __fildes, struct stat *__stat_buf);
 int __xstat (int __ver, const char *__filename,
 	     struct stat *__stat_buf);
 int __lxstat (int __ver, const char *__filename, struct stat *__stat_buf);
 int __fxstatat (int __ver, int __fildes, const char *__filename,
 		struct stat *__stat_buf, int __flag);
+#else
+int __REDIRECT_NTH (__fxstat,
+		(int __ver, int __fildes, struct stat *__stat_buf), __fxstat64);
+int __REDIRECT_NTH (__xstat,
+		(int __ver, const char *__filename,
+		 struct stat *__stat_buf), __xstat64);
+int __REDIRECT_NTH (__lxstat,
+		(int __ver, const char *__filename, struct stat *__stat_buf), __lxstat64);
+int __REDIRECT_NTH (__fxstatat,
+		(int __ver, int __fildes, const char *__filename,
+		 struct stat *__stat_buf, int __flag), __fxstatat64);
+#endif
 int __fxstat64 (int ver, int __fildes, struct stat64 *__stat_buf);
 int __xstat64 (int ver, const char *__filename, struct stat64 *__stat_buf);
 int __lxstat64 (int ver, const char *__filename, struct stat64 *__stat_buf);
-- 
2.28.0

Re: [PATCH] include/sys/stat.h: Add __USE_FILE_OFFSET64 redirection

[Adhemerval Zanella]

On 08/11/2020 13:42, Samuel Thibault wrote:
> Tests use include/sys/stat.h, which redirects the tests'
> {,l,f}stat calls to __{,l,f}xstat calls. When a test is specifying
> __USE_FILE_OFFSET64 (such as support/test-container.c), this
> becomes bogus because while {,l,f}stat have the 64bit redirection,
> __{,l,f}xstat did not have yet. This adds them.

The issue in fact the redefine at:

 97 # ifdef NO_RTLD_HIDDEN
 98 /* These are still required for Hurd.  */
[..]
111 #  define stat(fname, buf) __xstat (_STAT_VER, fname, buf)
112 #  define lstat(fname, buf)  __lxstat (_STAT_VER, fname, buf)
113 #  define __lstat(fname, buf)  __lxstat (_STAT_VER, fname, buf)
114 #  define lstat64(fname, buf)  __lxstat64 (_STAT_VER, fname, buf)
115 #  define __lstat64(fname, buf)  __lxstat64 (_STAT_VER, fname, buf)
116 #  define stat64(fname, buf) __xstat64 (_STAT_VER, fname, buf)
117 #  define __stat64(fname, buf) __xstat64 (_STAT_VER, fname, buf)
118 #  define fstat64(fd, buf) __fxstat64 (_STAT_VER, fd, buf)
119 #  define __fstat64(fd, buf) __fxstat64 (_STAT_VER, fd, buf)
120 #  define fstat(fd, buf) __fxstat (_STAT_VER, fd, buf)
121 #  define __fstat(fd, buf) __fxstat (_STAT_VER, fd, buf)
122 #  define __fstatat(dfd, fname, buf, flag) \
123     __fxstatat (_STAT_VER, dfd, fname, buf, flag)
124 #  define __fstatat64(dfd, fname, buf, flag) \
125     __fxstatat64 (_STAT_VER, dfd, fname, buf, flag)
[..]

Which is only active for Hurd.  On Linux, test-container build to:

[...]
extern int stat (const char *__restrict __file, struct stat *__restrict __buf) __asm__ ("" "stat64") __attribute__ ((__nothrow__ ))
[...]
void
maybe_xmkdir (const char *path, mode_t mode)
{ 
  struct stat st;

  if (stat (path, &st) == 0
      && ((((st.st_mode)) & 0170000) == (0040000)))
    return;
  xmkdir (path, mode);
}
[...]

As expected for internal and external interface.  And I am not sure
if moving a specific external interfaces (__REDIRECT_NTH) on internal
only header is the best solution.  At least *not* for generic code,
maybe by adding only for Hurd or if NO_RTLD_HIDDEN is defined at least.

But the question is why Hurd still need to access and use the old
__xstat interfaces, can we make Hurd use the versioned fstat interface
instead?

> ---
>  include/sys/stat.h | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/include/sys/stat.h b/include/sys/stat.h
> index 108cb0c9bf..692850740f 100644
> --- a/include/sys/stat.h
> +++ b/include/sys/stat.h
> @@ -29,14 +29,25 @@ in_blkcnt_t_range (__blkcnt64_t v)
>  }
>  
>  /* Now define the internal interfaces. */
> +#ifndef __USE_FILE_OFFSET64
>  extern int __stat (const char *__file, struct stat *__buf);
> -extern int __stat64 (const char *__file, struct stat64 *__buf);
>  extern int __fstat (int __fd, struct stat *__buf);
> -extern int __fstat64 (int __fd, struct stat64 *__buf);
>  extern int __lstat (const char *__file, struct stat *__buf);
> -extern int __lstat64 (const char *__file, struct stat64 *__buf);
>  extern int __fstatat (int dirfd, const char *pathname, struct stat *buf,
>  		      int flags);
> +#else
> +extern int __REDIRECT_NTH (__stat,
> +		(const char *__file, struct stat *__buf), __stat64);
> +extern int __REDIRECT_NTH (__fstat,
> +		(int __fd, struct stat *__buf), __fstat64);
> +extern int __REDIRECT_NTH (__lstat,
> +		(const char *__file, struct stat *__buf), __lstat64);
> +extern int __REDIRECT_NTH(__fstatat,
> +		(int dirfd, const char *pathname, struct stat *buf, int flags), __fstatat64);
> +#endif
> +extern int __stat64 (const char *__file, struct stat64 *__buf);
> +extern int __fstat64 (int __fd, struct stat64 *__buf);
> +extern int __lstat64 (const char *__file, struct stat64 *__buf);
>  extern int __fstatat64 (int dirfd, const char *pathname, struct stat64 *buf,
>  			int flags);
>  # if IS_IN (libc) || (IS_IN (rtld) && !defined NO_RTLD_HIDDEN)
> @@ -82,12 +93,25 @@ extern int __xmknod (int __ver, const char *__path, __mode_t __mode,
>  extern int __xmknodat (int __ver, int __fd, const char *__path,
>  		       __mode_t __mode, __dev_t *__dev);
>  
> +#ifndef __USE_FILE_OFFSET64
>  int __fxstat (int __ver, int __fildes, struct stat *__stat_buf);
>  int __xstat (int __ver, const char *__filename,
>  	     struct stat *__stat_buf);
>  int __lxstat (int __ver, const char *__filename, struct stat *__stat_buf);
>  int __fxstatat (int __ver, int __fildes, const char *__filename,
>  		struct stat *__stat_buf, int __flag);
> +#else
> +int __REDIRECT_NTH (__fxstat,
> +		(int __ver, int __fildes, struct stat *__stat_buf), __fxstat64);
> +int __REDIRECT_NTH (__xstat,
> +		(int __ver, const char *__filename,
> +		 struct stat *__stat_buf), __xstat64);
> +int __REDIRECT_NTH (__lxstat,
> +		(int __ver, const char *__filename, struct stat *__stat_buf), __lxstat64);
> +int __REDIRECT_NTH (__fxstatat,
> +		(int __ver, int __fildes, const char *__filename,
> +		 struct stat *__stat_buf, int __flag), __fxstatat64);
> +#endif
>  int __fxstat64 (int ver, int __fildes, struct stat64 *__stat_buf);
>  int __xstat64 (int ver, const char *__filename, struct stat64 *__stat_buf);
>  int __lxstat64 (int ver, const char *__filename, struct stat64 *__stat_buf);
> 

Re: [PATCH] include/sys/stat.h: Add __USE_FILE_OFFSET64 redirection

[Samuel Thibault]

Adhemerval Zanella, le lun. 09 nov. 2020 11:48:06 -0300, a ecrit:
> But the question is why Hurd still need to access and use the old
> __xstat interfaces, can we make Hurd use the versioned fstat interface
> instead?

I have no idea. I guess there is no reason, and it's just a leftover
that nobody took the time to clean. AFAIK there has never been
several versions of the stat structure, only the 32bit/64bit offset
stat/stat64. So AIUI there is no reason to keep the old xstat interface,
and we could as well simply rename all hurd *xstat implementations into
*stat, dropping the ver parameter and the redirection in the headers,
and reintroduce xstat stubs for ABI compatibility?

Samuel

Re: [PATCH] include/sys/stat.h: Add __USE_FILE_OFFSET64 redirection

[Florian Weimer]

* Samuel Thibault:

> Adhemerval Zanella, le lun. 09 nov. 2020 11:48:06 -0300, a ecrit:
>> But the question is why Hurd still need to access and use the old
>> __xstat interfaces, can we make Hurd use the versioned fstat interface
>> instead?
>
> I have no idea. I guess there is no reason, and it's just a leftover
> that nobody took the time to clean. AFAIK there has never been
> several versions of the stat structure, only the 32bit/64bit offset
> stat/stat64. So AIUI there is no reason to keep the old xstat interface,
> and we could as well simply rename all hurd *xstat implementations into
> *stat, dropping the ver parameter and the redirection in the headers,
> and reintroduce xstat stubs for ABI compatibility?

I agree that this would be a nice cleanup.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill