Cleanup stat shims

* Provide explicit constants for interpreting file mode
* Use extern "C" one function at a time
* Rename FileStats to FileStatus to match POSIX language
* Rename FileStatus to be more 1:1 with struct stat
* Put parameters on their own lines in header

Author: nguerrera

src/Common/src/Interop/Unix/System.Native/Interop.Stat.cs

@@ -8,9 +8,9 @@ internal static partial class Interop
 {
     internal static partial class System
     {
-        internal struct FileStats
+        internal struct FileStatus
         {
-            private FileStatsFlags Flags;
+            internal FileStatusFlags Flags;
             internal int Mode;
             internal int Uid;
             internal int Gid;
@@ -32,19 +32,19 @@ internal static class FileTypes
         }
 
         [Flags]
-        internal enum FileStatsFlags
+        internal enum FileStatusFlags
         {
             None = 0,
             HasCreationTime = 1,
         }
 
         [DllImport(Libraries.SystemNative, SetLastError = true)]
-        internal static extern int FStat(int fileDescriptor, out FileStats output);
+        internal static extern int FStat(int fileDescriptor, out FileStatus output);
 
         [DllImport(Libraries.SystemNative, SetLastError = true)]
-        internal static extern int Stat(string path, out FileStats output);
+        internal static extern int Stat(string path, out FileStatus output);
 
         [DllImport(Libraries.SystemNative, SetLastError = true)]
-        internal static extern int LStat(string path, out FileStats output);
+        internal static extern int LStat(string path, out FileStatus output);
     }
 }

src/Native/System.Native/pal_stat.cpp

@@ -1,7 +1,5 @@
-//
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
-//
 
 #include "../config.h"
 #include "pal_stat.h"
@@ -17,61 +15,89 @@
 #   define lstat_ lstat
 #endif
 
-static void ConvertFileStats(const struct stat_& src, FileStats* dst)
+// These numeric values are specified by POSIX.
+// Validate that our definitions match.
+static_assert(PAL_S_IRWXU == S_IRWXU, "");
+static_assert(PAL_S_IRUSR == S_IRUSR, "");
+static_assert(PAL_S_IWUSR == S_IWUSR, "");
+static_assert(PAL_S_IXUSR == S_IXUSR, "");
+static_assert(PAL_S_IRWXG == S_IRWXG, "");
+static_assert(PAL_S_IRGRP == S_IRGRP, "");
+static_assert(PAL_S_IWGRP == S_IWGRP, "");
+static_assert(PAL_S_IXGRP == S_IXGRP, "");
+static_assert(PAL_S_IRWXO == S_IRWXO, "");
+static_assert(PAL_S_IROTH == S_IROTH, "");
+static_assert(PAL_S_IWOTH == S_IWOTH, "");
+static_assert(PAL_S_IXOTH == S_IXOTH, "");
+static_assert(PAL_S_ISUID == S_ISUID, "");
+static_assert(PAL_S_ISGID == S_ISGID, "");
+
+// These numeric values are not specified by POSIX, but the values
+// are common to our current targets.  If these static asserts fail,
+// ConvertFileStatus needs to be updated to twiddle mode bits
+// accordingly.
+static_assert(PAL_S_IFMT  == S_IFMT,  "");
+static_assert(PAL_S_IFIFO == S_IFIFO, "");
+static_assert(PAL_S_IFCHR == S_IFCHR, "");
+static_assert(PAL_S_IFDIR == S_IFDIR, "");
+static_assert(PAL_S_IFREG == S_IFREG, "");
+static_assert(PAL_S_IFLNK == S_IFLNK, "");
+
+static void ConvertFileStatus(const struct stat_& src, FileStatus* dst)
 {
-    dst->Flags = FILESTATS_FLAGS_NONE;
-    dst->Mode = src.st_mode;
-    dst->Uid = src.st_uid;
-    dst->Gid = src.st_gid;
-    dst->Size = src.st_size;
-    dst->AccessTime = src.st_atime;
-    dst->ModificationTime = src.st_mtime;
-    dst->StatusChangeTime = src.st_ctime;
+    dst->Flags  = FILESTATUS_FLAGS_NONE;
+    dst->Mode   = src.st_mode;
+    dst->Uid    = src.st_uid;
+    dst->Gid    = src.st_gid;
+    dst->Size   = src.st_size;
+    dst->ATime  = src.st_atime;
+    dst->MTime  = src.st_mtime;
+    dst->CTime  = src.st_ctime;
 
 #if HAVE_STAT_BIRTHTIME
-    dst->CreationTime = src.st_birthtime;
-    dst->Flags |= FILESTATS_FLAGS_HAS_CREATION_TIME;
+    dst->BirthTime = src.st_birthtime;
+    dst->Flags |= FILESTATUSFLAGS_HAS_BIRTHTIME;
 #endif
 }
 
 extern "C"
+int32_t Stat(const char* path, FileStatus* output)
 {
-    int32_t Stat(const char* path, struct FileStats* output)
-    {
-        struct stat_ result;
-        int ret = stat_(path, &result);
-
-        if (ret == 0)
-        {
-            ConvertFileStats(result, output);
-        }
+    struct stat_ result;
+    int ret = stat_(path, &result);
 
-        return ret; // TODO: errno conversion
-    }
+     if (ret == 0)
+     {
+        ConvertFileStatus(result, output);
+     }
 
-    int32_t FStat(int32_t fileDescriptor, FileStats* output)
-    {
-        struct stat_ result;
-        int ret = fstat_(fileDescriptor, &result);
+    return ret;
+}
 
-        if (ret == 0)
-        {
-            ConvertFileStats(result, output);
-        }
+extern "C"
+int32_t FStat(int32_t fileDescriptor, FileStatus* output)
+{
+    struct stat_ result;
+    int ret = fstat_(fileDescriptor, &result);
 
-        return ret; // TODO: errno conversion
+    if (ret == 0)
+    {
+        ConvertFileStatus(result, output);
     }
 
-    int32_t LStat(const char* path, struct FileStats* output)
-    {
-        struct stat_ result;
-        int ret = lstat_(path, &result);
+    return ret;
+}
 
-        if (ret == 0)
-        {
-            ConvertFileStats(result, output);
-        }
+extern "C"
+int32_t LStat(const char* path, FileStatus* output)
+{
+    struct stat_ result;
+    int ret = lstat_(path, &result);
 
-        return ret; // TODO: errno conversion
+    if (ret == 0)
+    {
+        ConvertFileStatus(result, output);
     }
+
+    return ret;
 }

src/Native/System.Native/pal_stat.h

@@ -5,46 +5,94 @@
 
 #include <stdint.h>
 
-extern "C"
+/**
+ * File status returned by Stat or FStat.
+ */
+struct FileStatus
+{
+    int32_t Flags;     // flags for testing if some members are present (see FileStatusFlags)
+    int32_t Mode;      // file mode (see S_I* constants above for bit values)
+    int32_t Uid;       // user ID of owner
+    int32_t Gid;       // group ID of owner
+    int64_t Size;      // total size, in bytes
+    int64_t ATime;     // time of last access
+    int64_t MTime;     // time of last modification
+    int64_t CTime;     // time of last status change
+    int64_t BirthTime; // time the file was created
+};
+
+/**
+ * Constants for interpreting the permissions encoded in FileStatus.Mode.
+ * Both the names (without the PAL_ prefix and numeric values are specified by POSIX.1.2008
+ */
+enum
 {
-    struct FileStats
-    {
-        int32_t Flags;            // flags for testing if some members are present (see values below)
-        int32_t Mode;             // protection
-        int32_t Uid;              // user ID of owner
-        int32_t Gid;              // group ID of owner
-        int64_t Size;             // total size, in bytes
-        int64_t AccessTime;       // time of last access (atime)
-        int64_t ModificationTime; // time of last modification (mtime)
-        int64_t StatusChangeTime; // time of last status change (ctime)
-        int64_t CreationTime;     // time the file was created (birthtime)
-    };
-
-    enum
-    {
-        FILESTATS_FLAGS_NONE = 0,
-        FILESTATS_FLAGS_HAS_CREATION_TIME = 1,
-    };
-
-    /**
-     * Get file stats from a decriptor. Implemented as shim to fstat(2).
-     *
-     * Returns 0 for success, -1 for failure. Sets errno on failure.
-     */
-    int32_t FStat(int32_t fileDescriptor, FileStats* output);
-
-    /**
-     * Get file stats from a full path. Implemented as shim to stat(2).
-     *
-     * Returns 0 for success, -1 for failure. Sets errno on failure.
-     */
-    int32_t Stat(const char* path, FileStats* output);
-
-    /**
-    * Get file stats from a full path. Implemented as shim to lstat(2).
-    *
-    * Returns 0 for success, -1 for failure. Sets errno on failure.
-    */
-    int32_t LStat(const char* path, FileStats* output);
-}
+    PAL_S_IRWXU = 00700, // Read, write, execute/search by owner.
+    PAL_S_IRUSR = 00400, // Read permission, owner.
+    PAL_S_IWUSR = 00200, // Write permission, owner.
+    PAL_S_IXUSR = 00100, // Execute/search permission, owner.
+    PAL_S_IRWXG = 00070, // Read, write, execute/search by group.
+    PAL_S_IRGRP = 00040, // Read permission, group.
+    PAL_S_IWGRP = 00020, // Write permission, group.
+    PAL_S_IXGRP = 00010, // Execute/search permission, group.
+    PAL_S_IRWXO = 00007, // Read, write, execute/search by others.
+    PAL_S_IROTH = 00004, // Read permission, others.
+    PAL_S_IWOTH = 00002, // Write permission, others.
+    PAL_S_IXOTH = 00001, // Execute/search permission, others.
+    PAL_S_ISUID = 04000, // Set-user-ID on execution.
+    PAL_S_ISGID = 02000, // Set-group-ID on execution.
+};
 
+/**
+ * Constants for interpreting the permissions encoded in FileStatus.Mode
+ * Only the names (without the PAL_ prefix) are specified by POSIX.1.2008.
+ * The values chosen below are in common use, but not guaranteed.
+ */
+enum
+{
+    PAL_S_IFMT  = 0xF000, // Type of file (apply as mask to FileStatus.Mode and one of S_IF*)
+    PAL_S_IFIFO = 0x1000, // FIFO (named pipe)
+    PAL_S_IFCHR = 0x2000, // Character special
+    PAL_S_IFDIR = 0x4000, // Directory
+    PAL_S_IFREG = 0x8000, // Regular file
+    PAL_S_IFLNK = 0xA000, // Symbolic link
+};
+
+/**
+ * Constants for interpreting FileStatus.Flags.
+ */
+enum
+{
+    FILESTATUS_FLAGS_NONE          = 0,
+    FILESTATUS_FLAGS_HAS_BIRTHTIME = 1,
+};
+
+/**
+ * Get file status from a decriptor. Implemented as shim to fstat(2).
+ *
+ * Returns 0 for success, -1 for failure. Sets errno on failure.
+ */
+extern "C"
+int32_t FStat(
+    int32_t fileDescriptor,
+    FileStatus* output);
+
+/**
+ * Get file status from a full path. Implemented as shim to stat(2).
+ *
+ * Returns 0 for success, -1 for failure. Sets errno on failure.
+ */
+extern "C" 
+int32_t Stat(
+    const char* path,
+    FileStatus* output);
+
+/**
+ * Get file stats from a full path. Implemented as shim to lstat(2).
+ *
+ * Returns 0 for success, -1 for failure. Sets errno on failure.
+ */
+extern "C"
+int32_t LStat(
+    const char* path, 
+    FileStatus* output);

src/System.Console/src/System/ConsolePal.Unix.cs

@@ -378,7 +378,7 @@ internal UnixConsoleStream(string devPath, FileAccess access)
                 try
                 {
                     _handle.DangerousAddRef(ref gotFd);
-                    Interop.System.FileStats buf;
+                    Interop.System.FileStatus buf;
                     _handleType =
                         Interop.System.FStat((int)_handle.DangerousGetHandle(), out buf) == 0 ?
                             (buf.Mode & Interop.System.FileTypes.S_IFMT) :

src/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs

@@ -300,9 +300,9 @@ private WatchedDirectory AddDirectoryWatchUnlocked(WatchedDirectory parent, stri
                 string fullPath = parent != null ? parent.GetPath(false, directoryName) : directoryName;
 
                 // inotify_add_watch will fail if this is a symlink, so check that we didn't get a symlink
-                Interop.System.FileStats stats = default(Interop.System.FileStats);
-                if ((Interop.System.LStat(fullPath, out stats) == 0) &&
-                    ((stats.Mode & (uint)Interop.System.FileTypes.S_IFMT) == Interop.System.FileTypes.S_IFLNK))
+                Interop.System.FileStatus status = default(Interop.System.FileStatus);
+                if ((Interop.System.LStat(fullPath, out status) == 0) &&
+                    ((status.Mode & (uint)Interop.System.FileTypes.S_IFMT) == Interop.System.FileTypes.S_IFLNK))
                 {
                     return null;
                 }