Respond to PR feedback

* Rename shims to match their underlying platform API names
* Update guidelines
* Add errno conversion TODO
* Account for OS X in run-test.sh

Author: nguerrera

Documentation/coding-guidelines/interop-guidelines.md

@@ -254,8 +254,10 @@ rather than the underlying integral type.
 
 Often, various UNIX flavors offer the same API from the point-of-view of compatibility
 with C/C++ source code, but they do not have the same ABI. e.g. Fields can be laid out
-differently, constants can have different numeric values, etc. Even the exports can
-be named differently.
+differently, constants cn have different numeric values, exports can
+be named differently, etc. There are not only differences between operating systems
+(Mac OS X vs. Ubuntu vs. FreeBSD), but also differences related to the underlying
+processor architecture (x64 vs. x86 vs. ARM).
 
 This leaves us with a situation where we can't write portable P/Invoke declarations
 that will work on all flavors, and writing separate declarations per flavor is quite
@@ -267,10 +269,6 @@ a P/Invoke to a C++ lib written specifically for corefx. These libs -- System.*.
 Generally, they are not there to add any significant abstraction, but to create a 
 stable ABI such that the same IL assembly can work across UNIX flavors.
 
-At this time, these shims are compiled in the dotnet/coreclr repository under the corefx
-folder. This is temporary (issue #2301) until we add necessary infrastructure to build them 
-in this repository. 
-
 Guidelines for shim C++ API:
 
 - Keep them as "thin"/1:1 as possible. 
@@ -279,12 +277,14 @@ Guidelines for shim C++ API:
 easy to assume something is safe/guaranteed when it isn't.
 - Don't cheat and take advantage of coincidental agreement between
 one flavor's ABI and the shim's ABI. 
-- Use PascalCase and spell things out in a style closer to Win32 than libc. 
-  - At first, it seemed that we'd want to use 1:1 names for the shims, but it 
-    turns out there are many cases where being strictly 1:1 isn't practical. As such, 
-    the libraries will end up looking more self-consistent if we give them their
-    own style with which to express themselves.
-- Stick to data types which are guaranteed not to vary in size across flavors. (Pointers
-and size_t variance across bitness is OK.) 
-  - e.g. use int32_t, int64_t from stdint.h and not int, long. 
+- Use PascalCase in a style closer to Win32 than libc.
+  - If an export point has a 1:1 correspondence to the platform API, then name
+    it after the platform API in PascalCase (e.g. stat -> Stat, fstat -> FStat).
+  - If an export is not 1:1, then spell things out as we typically would in
+    CoreFX code (i.e. don't use abbreviations unless they come from the underlying
+    API.
+  - At first, it seemed that we'd want to use 1:1 names throughout, but it
+    turns out there are many cases where being strictly 1:1 isn't practical.
+- Stick to data types which are guaranteed not to vary in size across flavors.
+  - e.g. use int32_t, int64_t from stdint.h and not int, long.
 

run-test.sh

@@ -129,11 +129,11 @@ create_test_overlay()
   fi
   find $CoreFxBins -name '*.dll' -exec cp '{}' "$OverlayDir" ";"
 
-  # Then the native linux CoreFX binaries
+  # Then the native CoreFX binaries
   #
   # TODO: Currently, CI does not build the native CoreFX components so build them here
   #       in the test phase for now.
-  ( $ProjectRoot/src/Native/build.sh && cp $ProjectRoot/bin/$OS.x64.$Configuration/Native/*.so $OverlayDir ) || exit 1
+  ( $ProjectRoot/src/Native/build.sh && cp $ProjectRoot/bin/$OS.x64.$Configuration/Native/* $OverlayDir ) || exit 1
 }
 
 copy_test_overlay()

src/Common/src/Interop/Unix/System.IO.Native/Interop.NativeIO.cs

@@ -39,9 +39,9 @@ internal enum FileStatsFlags
         }
 
         [DllImport(Libraries.IOInterop, SetLastError = true)]
-        internal static extern int GetFileStatsFromDescriptor(int fileDescriptor, out FileStats output);
+        internal static extern int FStat(int fileDescriptor, out FileStats output);
 
         [DllImport(Libraries.IOInterop, SetLastError = true)]
-        internal static extern int GetFileStatsFromPath(string path, out FileStats output);
+        internal static extern int Stat(string path, out FileStats output);
     }
 }

src/Native/System.IO.Native/nativeio.cpp

@@ -33,7 +33,7 @@ static void ConvertFileStats(const struct stat_& src, FileStats* dst)
 
 extern "C"
 {
-    int32_t GetFileStatsFromPath(const char* path, struct FileStats* output)
+    int32_t Stat(const char* path, struct FileStats* output)
     {
         struct stat_ result;
         int ret = stat_(path, &result);
@@ -43,10 +43,10 @@ extern "C"
             ConvertFileStats(result, output);
         }
 
-        return ret;
+        return ret; // TODO: errno conversion
     }
 
-    int32_t GetFileStatsFromDescriptor(int32_t fileDescriptor, FileStats* output)
+    int32_t FStat(int32_t fileDescriptor, FileStats* output)
     {
         struct stat_ result;
         int ret = fstat_(fileDescriptor, &result);
@@ -56,6 +56,6 @@ extern "C"
             ConvertFileStats(result, output);
         }
 
-        return ret;
+        return ret; // TODO: errno conversion
     }
 }

src/Native/System.IO.Native/nativeio.h

@@ -33,13 +33,13 @@ extern "C"
      *
      * Returns 0 for success, -1 for failure. Sets errno on failure.
      */
-    int32_t GetFileStatsFromDescriptor(int32_t fileDescriptor, FileStats* output);
+    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 GetFileStatsFromPath(const char* path, FileStats* output);
+    int32_t Stat(const char* path, FileStats* output);
 }
 

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

@@ -380,7 +380,7 @@ internal UnixConsoleStream(string devPath, FileAccess access)
                     _handle.DangerousAddRef(ref gotFd);
                     Interop.NativeIO.FileStats buf;
                     _handleType =
-                        Interop.NativeIO.GetFileStatsFromDescriptor((int)_handle.DangerousGetHandle(), out buf) == 0 ?
+                        Interop.NativeIO.FStat((int)_handle.DangerousGetHandle(), out buf) == 0 ?
                             (buf.Mode & Interop.NativeIO.FileTypes.S_IFMT) :
                             Interop.NativeIO.FileTypes.S_IFREG; // if something goes wrong, don't fail, just say it's a regular file
                 }