From 335b1f8553107a73077ae0a2dcf094911d390ece Mon Sep 17 00:00:00 2001 From: "R. Fontenot" Date: Sun, 14 Aug 2016 23:56:00 -0500 Subject: [PATCH 1/5] Replace readdir_r() with readdir() readdir_r() is deprecated in glibc >= v2.24 --- src/Native/System.Native/pal_io.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Native/System.Native/pal_io.cpp b/src/Native/System.Native/pal_io.cpp index 7da1bff..9bd0ec8 100644 --- a/src/Native/System.Native/pal_io.cpp +++ b/src/Native/System.Native/pal_io.cpp @@ -339,27 +339,28 @@ extern "C" int32_t SystemNative_ReadDirR(DIR* dir, void* buffer, int32_t bufferS return ERANGE; } - dirent* result = nullptr; - dirent* entry = static_cast(buffer); - int error = readdir_r(dir, entry, &result); + // readdir returns a pointer to memory that may be staticly allocated + // by glibc. Data returned by readdir may be overwritten by other readdir + // calls for the same directory stream. + errno = 0; + dirent* entry = readdir(dir); // positive error number returned -> failure - if (error != 0) + if (errno != 0) { - assert(error > 0); + assert(error == EBADF); // Invalid directory stream discriptor dir. *outputEntry = {}; // managed out param must be initialized - return error; + return errno; } // 0 returned with null result -> end-of-stream - if (result == nullptr) + if (entry == nullptr) { *outputEntry = {}; // managed out param must be initialized return -1; // shim convention for end-of-stream } - // 0 returned with non-null result (guaranteed to be set to entry arg) -> success - assert(result == entry); + memcpy(buffer,entry,static_cast(bufferSize)); ConvertDirent(*entry, outputEntry); return 0; } -- 2.9.2 From a74fa9876a8479e206a3357c481ffbe574b9fa9c Mon Sep 17 00:00:00 2001 From: "R. Fontenot" Date: Mon, 15 Aug 2016 10:42:54 -0500 Subject: [PATCH 2/5] Clean up discrepancies in the documentation. --- src/Native/System.Native/pal_io.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Native/System.Native/pal_io.cpp b/src/Native/System.Native/pal_io.cpp index 9bd0ec8..6977f0d 100644 --- a/src/Native/System.Native/pal_io.cpp +++ b/src/Native/System.Native/pal_io.cpp @@ -321,12 +321,15 @@ extern "C" int32_t SystemNative_GetDirentSize() // size of the dirent struct. // 2) The managed code creates a byte[] buffer of the size of the native dirent // and passes a pointer to this buffer to this function. -// 3) This function passes input byte[] buffer to the OS to fill with dirent data -// which makes the 1st strcpy. -// 4) The ConvertDirent function will set a pointer to the start of the inode name -// in the byte[] buffer so the managed code and find it and copy it out of the +// 3) This function gets a pointer to the possibly staticly allocated directory entry. +// 4) Then, byte[] entry is copied into the byte[] buffer for bufferSize bytes. +// This makes the 1st strcpy. +// 5) The ConvertDirent function will set a pointer to the start of the inode name +// in the byte[] buffer so the managed code can find it and copy it out of the // buffer into a managed string that the caller of the framework can use, making // the 2nd and final strcpy. +// +// To make this function thread safe ensure calls on the same DIR* dir never happen concurrently. extern "C" int32_t SystemNative_ReadDirR(DIR* dir, void* buffer, int32_t bufferSize, DirectoryEntry* outputEntry) { assert(buffer != nullptr); @@ -339,10 +342,10 @@ extern "C" int32_t SystemNative_ReadDirR(DIR* dir, void* buffer, int32_t bufferS return ERANGE; } - // readdir returns a pointer to memory that may be staticly allocated - // by glibc. Data returned by readdir may be overwritten by other readdir - // calls for the same directory stream. errno = 0; + // returns a pointer to memory that may be staticly allocated by glibc. + // Data returned by readdir may be overwritten by other readdir calls + // on the same directory stream. dirent* entry = readdir(dir); // positive error number returned -> failure -- 2.9.2 From 53f6a769d1a1269b4936cbd4dc7b359d48729ed7 Mon Sep 17 00:00:00 2001 From: "R. Fontenot" Date: Tue, 16 Aug 2016 09:36:08 -0500 Subject: [PATCH 3/5] Add readdir_r cxx_check_source_compiles --- src/Native/configure.cmake | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Native/configure.cmake b/src/Native/configure.cmake index 017d75f..abc0360 100644 --- a/src/Native/configure.cmake +++ b/src/Native/configure.cmake @@ -174,6 +174,20 @@ check_cxx_source_compiles( check_cxx_source_compiles( " + #include + int main(void) + { + DIR* dir; + struct dirent* entry; + struct dirent* result; + readdir_r(dir,entry,&result); + return 0; + } + " + HAVE_GNU_READDIR_R) + +check_cxx_source_compiles( + " #include #include int main(void) -- 2.9.2 From ed8510abf9cebb4d0d5d00c28bfb355768bb0c9b Mon Sep 17 00:00:00 2001 From: "R. Fontenot" Date: Tue, 16 Aug 2016 09:54:38 -0500 Subject: [PATCH 4/5] Add copy of SystemNative_ReadDirR() from master --- src/Native/System.Native/pal_io.cpp | 51 ++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/Native/System.Native/pal_io.cpp b/src/Native/System.Native/pal_io.cpp index 6977f0d..b7908b5 100644 --- a/src/Native/System.Native/pal_io.cpp +++ b/src/Native/System.Native/pal_io.cpp @@ -316,6 +316,56 @@ extern "C" int32_t SystemNative_GetDirentSize() return sizeof(dirent); } +#if defined HAVE_GNU_READDIR_R +// To reduce the number of string copies, this function calling pattern works as follows: +// 1) The managed code calls GetDirentSize() to get the platform-specific +// size of the dirent struct. +// 2) The managed code creates a byte[] buffer of the size of the native dirent +// and passes a pointer to this buffer to this function. +// 3) This function passes input byte[] buffer to the OS to fill with dirent data +// which makes the 1st strcpy. +// 4) The ConvertDirent function will set a pointer to the start of the inode name +// in the byte[] buffer so the managed code and find it and copy it out of the +// buffer into a managed string that the caller of the framework can use, making +// the 2nd and final strcpy. +extern "C" int32_t SystemNative_ReadDirR(DIR* dir, void* buffer, int32_t bufferSize, DirectoryEntry* outputEntry) +{ + assert(buffer != nullptr); + assert(dir != nullptr); + assert(outputEntry != nullptr); + + if (bufferSize < static_cast(sizeof(dirent))) + { + assert(false && "Buffer size too small; use GetDirentSize to get required buffer size"); + return ERANGE; + } + + dirent* result = nullptr; + dirent* entry = static_cast(buffer); + int error = readdir_r(dir, entry, &result); + + // positive error number returned -> failure + if (error != 0) + { + assert(error > 0); + *outputEntry = {}; // managed out param must be initialized + return error; + } + + // 0 returned with null result -> end-of-stream + if (result == nullptr) + { + *outputEntry = {}; // managed out param must be initialized + return -1; // shim convention for end-of-stream + } + + // 0 returned with non-null result (guaranteed to be set to entry arg) -> success + assert(result == entry); + ConvertDirent(*entry, outputEntry); + return 0; +} + +#else // To reduce the number of string copies, this function calling pattern works as follows: // 1) The managed code calls GetDirentSize() to get the platform-specific // size of the dirent struct. @@ -367,6 +417,7 @@ extern "C" int32_t SystemNative_ReadDirR(DIR* dir, void* buffer, int32_t bufferS ConvertDirent(*entry, outputEntry); return 0; } +#endif extern "C" DIR* SystemNative_OpenDir(const char* path) { -- 2.9.2 From 59bd06622e1d0894954ce7e625ccf019953831e8 Mon Sep 17 00:00:00 2001 From: "R. Fontenot" Date: Tue, 16 Aug 2016 09:56:47 -0500 Subject: [PATCH 5/5] Fix assert --- src/Native/System.Native/pal_io.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Native/System.Native/pal_io.cpp b/src/Native/System.Native/pal_io.cpp index b7908b5..e5e7a4f 100644 --- a/src/Native/System.Native/pal_io.cpp +++ b/src/Native/System.Native/pal_io.cpp @@ -401,7 +401,7 @@ extern "C" int32_t SystemNative_ReadDirR(DIR* dir, void* buffer, int32_t bufferS // positive error number returned -> failure if (errno != 0) { - assert(error == EBADF); // Invalid directory stream discriptor dir. + assert(errno == EBADF); // Invalid directory stream discriptor dir. *outputEntry = {}; // managed out param must be initialized return errno; } -- 2.9.2