[glib/wip/smcv/statx-no-required-mask: 1/2] glocalfile: Remove the concept of the required mask

[Simon McVittie <smcv src gnome org>]

commit b6ec6f786872306facdd1817879dbf1c5d5be696
Author: Simon McVittie <smcv collabora com>
Date:   Tue Sep 15 10:35:08 2020 +0100

    glocalfile: Remove the concept of the required mask
    
    It is not actually guaranteed that *any* of the flags in the
    returned stx_mask will be set by statx(): for example, if the
    filesystem has no concept of POSIX user and group IDs (like FAT),
    the kernel could validly clear the STATX_UID and STATX_GID bits in
    the returned stx_mask. Linux 5.8 does not appear to do this in practice,
    even for FAT, but the statx(2) man page suggests that it perhaps should.
    
    However, what *is* guaranteed is that for the STATX_BASIC_STATS (those
    that correspond to what is in the traditional struct stat), even if the
    bit in stx_mask is cleared, there will be *something* in the
    corresponding struct field (in practice the same harmless value that
    traditional stat() would have placed in the struct stat). Rely on those
    compatibility values being reasonable, and don't second-guess the kernel.
    
    Signed-off-by: Simon McVittie <smcv collabora com>

 gio/glocalfile.c             |  6 ++---
 gio/glocalfileinfo.c         |  3 ---
 gio/glocalfileinfo.h         | 61 ++++----------------------------------------
 gio/glocalfileoutputstream.c | 30 ++++++++++++++++------
 4 files changed, 29 insertions(+), 71 deletions(-)
---
diff --git a/gio/glocalfile.c b/gio/glocalfile.c
index a87de9cc4..fe1fc9ef7 100644
--- a/gio/glocalfile.c
+++ b/gio/glocalfile.c
@@ -1352,7 +1352,7 @@ g_local_file_read (GFile         *file,
       return NULL;
     }
 
-  ret = g_local_file_fstat (fd, G_LOCAL_FILE_STAT_FIELD_TYPE, G_LOCAL_FILE_STAT_FIELD_ALL, &buf);
+  ret = g_local_file_fstat (fd, G_LOCAL_FILE_STAT_FIELD_TYPE, &buf);
 
   if (ret == 0 && S_ISDIR (_g_stat_mode (&buf)))
     {
@@ -2703,9 +2703,7 @@ g_local_file_measure_size_of_file (gint           parent_fd,
 
 #if defined (AT_FDCWD)
   if (g_local_file_fstatat (parent_fd, name->data, AT_SYMLINK_NOFOLLOW,
-                            G_LOCAL_FILE_STAT_FIELD_BASIC_STATS,
-                            G_LOCAL_FILE_STAT_FIELD_ALL & (~G_LOCAL_FILE_STAT_FIELD_ATIME),
-                            &buf) != 0)
+                            G_LOCAL_FILE_STAT_FIELD_BASIC_STATS, &buf) != 0)
     {
       int errsv = errno;
       return g_local_file_measure_size_error (state->flags, errsv, name, error);
diff --git a/gio/glocalfileinfo.c b/gio/glocalfileinfo.c
index 90fcb3336..b9c099cac 100644
--- a/gio/glocalfileinfo.c
+++ b/gio/glocalfileinfo.c
@@ -1808,7 +1808,6 @@ _g_local_file_info_get (const char             *basename,
 
   res = g_local_file_lstat (path,
                             G_LOCAL_FILE_STAT_FIELD_BASIC_STATS | G_LOCAL_FILE_STAT_FIELD_BTIME,
-                            G_LOCAL_FILE_STAT_FIELD_ALL & (~G_LOCAL_FILE_STAT_FIELD_BTIME) & 
(~G_LOCAL_FILE_STAT_FIELD_ATIME),
                             &statbuf);
 
   if (res == -1)
@@ -1858,7 +1857,6 @@ _g_local_file_info_get (const char             *basename,
        {
           res = g_local_file_stat (path,
                                    G_LOCAL_FILE_STAT_FIELD_BASIC_STATS | G_LOCAL_FILE_STAT_FIELD_BTIME,
-                                   G_LOCAL_FILE_STAT_FIELD_ALL & (~G_LOCAL_FILE_STAT_FIELD_BTIME) & 
(~G_LOCAL_FILE_STAT_FIELD_ATIME),
                                    &statbuf2);
 
          /* Report broken links as symlinks */
@@ -2081,7 +2079,6 @@ _g_local_file_info_get_from_fd (int         fd,
 
   if (g_local_file_fstat (fd,
                           G_LOCAL_FILE_STAT_FIELD_BASIC_STATS | G_LOCAL_FILE_STAT_FIELD_BTIME,
-                          G_LOCAL_FILE_STAT_FIELD_ALL & (~G_LOCAL_FILE_STAT_FIELD_BTIME) & 
(~G_LOCAL_FILE_STAT_FIELD_ATIME),
                           &stat_buf) == -1)
     {
       int errsv = errno;
diff --git a/gio/glocalfileinfo.h b/gio/glocalfileinfo.h
index f2beb70cd..53ccd7a84 100644
--- a/gio/glocalfileinfo.h
+++ b/gio/glocalfileinfo.h
@@ -80,33 +80,17 @@ g_local_file_statx (int                  dirfd,
                     const char          *pathname,
                     int                  flags,
                     GLocalFileStatField  mask,
-                    GLocalFileStatField  mask_required,
                     GLocalFileStat      *stat_buf)
 {
-  int retval;
-
-  /* Allow the caller to set mask_required==G_LOCAL_FILE_STAT_FIELD_ALL as a
-   * shortcut for saying it’s equal to @mask. */
-  mask_required &= mask;
-
-  retval = statx (dirfd, pathname, flags, mask, stat_buf);
-  if (retval == 0 && (stat_buf->stx_mask & mask_required) != mask_required)
-    {
-      /* Not all required fields could be returned. */
-      errno = ERANGE;
-      return -1;
-    }
-
-  return retval;
+  return statx (dirfd, pathname, flags, mask, stat_buf);
 }
 
 static inline int
 g_local_file_fstat (int                  fd,
                     GLocalFileStatField  mask,
-                    GLocalFileStatField  mask_required,
                     GLocalFileStat      *stat_buf)
 {
-  return g_local_file_statx (fd, "", AT_EMPTY_PATH, mask, mask_required, stat_buf);
+  return g_local_file_statx (fd, "", AT_EMPTY_PATH, mask, stat_buf);
 }
 
 static inline int
@@ -114,32 +98,29 @@ g_local_file_fstatat (int                  fd,
                       const char          *path,
                       int                  flags,
                       GLocalFileStatField  mask,
-                      GLocalFileStatField  mask_required,
                       GLocalFileStat      *stat_buf)
 {
-  return g_local_file_statx (fd, path, flags, mask, mask_required, stat_buf);
+  return g_local_file_statx (fd, path, flags, mask, stat_buf);
 }
 
 static inline int
 g_local_file_lstat (const char          *path,
                     GLocalFileStatField  mask,
-                    GLocalFileStatField  mask_required,
                     GLocalFileStat      *stat_buf)
 {
   return g_local_file_statx (AT_FDCWD, path,
                              AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW | AT_STATX_SYNC_AS_STAT,
-                             mask, mask_required, stat_buf);
+                             mask, stat_buf);
 }
 
 static inline int
 g_local_file_stat (const char          *path,
                    GLocalFileStatField  mask,
-                   GLocalFileStatField  mask_required,
                    GLocalFileStat      *stat_buf)
 {
   return g_local_file_statx (AT_FDCWD, path,
                              AT_NO_AUTOMOUNT | AT_STATX_SYNC_AS_STAT,
-                             mask, mask_required, stat_buf);
+                             mask, stat_buf);
 }
 
 inline static gboolean _g_stat_has_field  (const GLocalFileStat *buf, GLocalFileStatField field) { return 
buf->stx_mask & field; }
@@ -204,16 +185,8 @@ typedef enum
 static inline int
 g_local_file_fstat (int                  fd,
                     GLocalFileStatField  mask,
-                    GLocalFileStatField  mask_required,
                     GLocalFileStat      *stat_buf)
 {
-  if ((G_LOCAL_FILE_STAT_FIELD_BASIC_STATS & (mask_required & mask)) != (mask_required & mask))
-    {
-      /* Only G_LOCAL_FILE_STAT_FIELD_BASIC_STATS are supported. */
-      errno = ERANGE;
-      return -1;
-    }
-
 #ifdef G_OS_WIN32
   return GLIB_PRIVATE_CALL (g_win32_fstat) (fd, stat_buf);
 #else
@@ -226,16 +199,8 @@ g_local_file_fstatat (int                  fd,
                       const char          *path,
                       int                  flags,
                       GLocalFileStatField  mask,
-                      GLocalFileStatField  mask_required,
                       GLocalFileStat      *stat_buf)
 {
-  if ((G_LOCAL_FILE_STAT_FIELD_BASIC_STATS & (mask_required & mask)) != (mask_required & mask))
-    {
-      /* Only G_LOCAL_FILE_STAT_FIELD_BASIC_STATS are supported. */
-      errno = ERANGE;
-      return -1;
-    }
-
 #ifdef G_OS_WIN32
   /* Currently not supported on Windows */
   errno = ENOSYS;
@@ -248,16 +213,8 @@ g_local_file_fstatat (int                  fd,
 static inline int
 g_local_file_lstat (const char          *path,
                     GLocalFileStatField  mask,
-                    GLocalFileStatField  mask_required,
                     GLocalFileStat      *stat_buf)
 {
-  if ((G_LOCAL_FILE_STAT_FIELD_BASIC_STATS & (mask_required & mask)) != (mask_required & mask))
-    {
-      /* Only G_LOCAL_FILE_STAT_FIELD_BASIC_STATS are supported. */
-      errno = ERANGE;
-      return -1;
-    }
-
 #ifdef G_OS_WIN32
   return GLIB_PRIVATE_CALL (g_win32_lstat_utf8) (path, stat_buf);
 #else
@@ -268,16 +225,8 @@ g_local_file_lstat (const char          *path,
 static inline int
 g_local_file_stat (const char          *path,
                    GLocalFileStatField  mask,
-                   GLocalFileStatField  mask_required,
                    GLocalFileStat      *stat_buf)
 {
-  if ((G_LOCAL_FILE_STAT_FIELD_BASIC_STATS & (mask_required & mask)) != (mask_required & mask))
-    {
-      /* Only G_LOCAL_FILE_STAT_FIELD_BASIC_STATS are supported. */
-      errno = ERANGE;
-      return -1;
-    }
-
 #ifdef G_OS_WIN32
   return GLIB_PRIVATE_CALL (g_win32_stat_utf8) (path, stat_buf);
 #else
diff --git a/gio/glocalfileoutputstream.c b/gio/glocalfileoutputstream.c
index f34c3e439..a28ac332c 100644
--- a/gio/glocalfileoutputstream.c
+++ b/gio/glocalfileoutputstream.c
@@ -429,7 +429,8 @@ _g_local_file_output_stream_really_close (GLocalFileOutputStream *file,
       
 #ifndef G_OS_WIN32             /* Already did the fstat() and close() above on Win32 */
 
-  if (g_local_file_fstat (file->priv->fd, G_LOCAL_FILE_STAT_FIELD_MTIME, G_LOCAL_FILE_STAT_FIELD_ALL, 
&final_stat) == 0)
+  if (g_local_file_fstat (file->priv->fd, G_LOCAL_FILE_STAT_FIELD_MTIME, &final_stat) == 0 &&
+      _g_stat_has_field (&final_stat, G_LOCAL_FILE_STAT_FIELD_MTIME))
     file->priv->etag = _g_local_file_info_create_etag (&final_stat);
 
   if (!g_close (file->priv->fd, NULL))
@@ -904,7 +905,7 @@ handle_overwrite_open (const char    *filename,
                             G_LOCAL_FILE_STAT_FIELD_GID |
                             G_LOCAL_FILE_STAT_FIELD_MTIME |
                             G_LOCAL_FILE_STAT_FIELD_NLINK,
-                            G_LOCAL_FILE_STAT_FIELD_ALL, &original_stat);
+                            &original_stat);
   errsv = errno;
 
   if (res != 0)
@@ -919,9 +920,11 @@ handle_overwrite_open (const char    *filename,
     }
   
   /* not a regular file */
-  if (!S_ISREG (_g_stat_mode (&original_stat)))
+  if (!_g_stat_has_field (&original_stat, G_LOCAL_FILE_STAT_FIELD_TYPE) ||
+      !S_ISREG (_g_stat_mode (&original_stat)))
     {
-      if (S_ISDIR (_g_stat_mode (&original_stat)))
+      if (_g_stat_has_field (&original_stat, G_LOCAL_FILE_STAT_FIELD_TYPE) &&
+          S_ISDIR (_g_stat_mode (&original_stat)))
        g_set_error_literal (error,
                              G_IO_ERROR,
                              G_IO_ERROR_IS_DIRECTORY,
@@ -936,6 +939,8 @@ handle_overwrite_open (const char    *filename,
   
   if (etag != NULL)
     {
+      /* TODO: What if original_stat doesn't have the mtime, or whatever
+       * other information goes into the etag? */
       current_etag = _g_local_file_info_create_etag (&original_stat);
       if (strcmp (etag, current_etag) != 0)
        {
@@ -961,7 +966,8 @@ handle_overwrite_open (const char    *filename,
    */
   
   if ((flags & G_FILE_CREATE_REPLACE_DESTINATION) ||
-      (!(_g_stat_nlink (&original_stat) > 1) && !is_symlink))
+      (!(_g_stat_has_field (&original_stat, G_LOCAL_FILE_STAT_FIELD_NLINK) &&
+         _g_stat_nlink (&original_stat) > 1) && !is_symlink))
     {
       char *dirname, *tmp_filename;
       int tmpfd;
@@ -979,6 +985,7 @@ handle_overwrite_open (const char    *filename,
       
       /* try to keep permissions (unless replacing) */
 
+      /* TODO: Check for G_LOCAL_FILE_STAT_FIELD_UID, _GID, _MODE */
       if ( ! (flags & G_FILE_CREATE_REPLACE_DESTINATION) &&
           (
 #ifdef HAVE_FCHOWN
@@ -999,9 +1006,10 @@ handle_overwrite_open (const char    *filename,
                                      G_LOCAL_FILE_STAT_FIELD_MODE |
                                      G_LOCAL_FILE_STAT_FIELD_UID |
                                      G_LOCAL_FILE_STAT_FIELD_GID,
-                                     G_LOCAL_FILE_STAT_FIELD_ALL, &tmp_statbuf);
+                                     &tmp_statbuf);
 
          /* Check that we really needed to change something */
+         /* TODO: Check for G_LOCAL_FILE_STAT_FIELD_UID, _GID, _MODE */
          if (tres != 0 ||
              _g_stat_uid (&original_stat) != _g_stat_uid (&tmp_statbuf) ||
              _g_stat_gid (&original_stat) != _g_stat_gid (&tmp_statbuf) ||
@@ -1041,6 +1049,9 @@ handle_overwrite_open (const char    *filename,
          goto err_out;
        }
 
+      /* TODO: Check for G_LOCAL_FILE_STAT_FIELD_MODE? (But if it's
+       * unset, we have to specify *something*: will our fallback be
+       * any better than the kernel's?) */
       bfd = g_open (backup_filename,
                    O_WRONLY | O_CREAT | O_EXCL | O_BINARY,
                    _g_stat_mode (&original_stat) & 0777);
@@ -1060,7 +1071,7 @@ handle_overwrite_open (const char    *filename,
        * bits for the group same as the protection bits for
        * others. */
 #if defined(HAVE_FCHOWN) && defined(HAVE_FCHMOD)
-      if (g_local_file_fstat (bfd, G_LOCAL_FILE_STAT_FIELD_GID, G_LOCAL_FILE_STAT_FIELD_ALL, &tmp_statbuf) 
!= 0)
+      if (g_local_file_fstat (bfd, G_LOCAL_FILE_STAT_FIELD_GID, &tmp_statbuf) != 0)
        {
          g_set_error_literal (error,
                                G_IO_ERROR,
@@ -1072,9 +1083,12 @@ handle_overwrite_open (const char    *filename,
          goto err_out;
        }
       
-      if ((_g_stat_gid (&original_stat) != _g_stat_gid (&tmp_statbuf))  &&
+      if (_g_stat_has_field (&tmp_statbuf, G_LOCAL_FILE_STAT_FIELD_GID) &&
+          _g_stat_has_field (&original_stat, G_LOCAL_FILE_STAT_FIELD_GID) &&
+          (_g_stat_gid (&original_stat) != _g_stat_gid (&tmp_statbuf)) &&
          fchown (bfd, (uid_t) -1, _g_stat_gid (&original_stat)) != 0)
        {
+         /* TODO: Check for G_LOCAL_FILE_STAT_FIELD_MODE? */
          if (fchmod (bfd,
                      (_g_stat_mode (&original_stat) & 0707) |
                      ((_g_stat_mode (&original_stat) & 07) << 3)) != 0)