[Fix] Added API mapping for fclose() and fileno().

enricogior · enricogior · commit 7b920de23126 · 2015-07-15T13:07:26.000+02:00
This fix resolves issue microsoftarchive#235 microsoftarchive#235
diff --git a/src/Win32_Interop/Win32_FDAPI.cpp b/src/Win32_Interop/Win32_FDAPI.cpp
@@ -51,6 +51,8 @@ redis_WSASocket WSASocket = NULL;
 
 // other API forwards
 redis_fwrite fdapi_fwrite = NULL;
+redis_fclose fdapi_fclose = NULL;
+redis_fileno fdapi_fileno = NULL;
 redis_setmode fdapi_setmode = NULL;
 redis_select select = NULL;
 redis_ntohl ntohl = NULL;
@@ -671,33 +673,26 @@ ssize_t redis_write_impl(int fd, const void *buf, size_t count) {
 int redis_fsync_impl(int fd) {
     try {
         int posixFD = RFDMap::getInstance().lookupPosixFD( fd );
-        if( posixFD == -1 ) {
-            // There is one place in Redis where we are not tracking posix FDs because it involves
-            // direct ocnversion of a FILE* to an FD.
-            posixFD = fd;
-        }
-
-        HANDLE h = (HANDLE) crtget_osfhandle(posixFD);
-        DWORD err;
-
-        if (h == INVALID_HANDLE_VALUE) {
-            errno = EBADF;
-            return -1;
-        }
+        if (posixFD != -1) {
+            HANDLE h = (HANDLE) crtget_osfhandle(posixFD);
+            if (h == INVALID_HANDLE_VALUE) {
+                errno = EBADF;
+                return -1;
+            }
 
-        if (!FlushFileBuffers(h)) {
-            err = GetLastError();
-            switch (err) {
-                case ERROR_INVALID_HANDLE:
-                errno = EINVAL;
-                break;
+            if (!FlushFileBuffers(h)) {
+                DWORD err = GetLastError();
+                switch (err) {
+                    case ERROR_INVALID_HANDLE:
+                        errno = EINVAL;
+                        break;
 
-                default:
-                errno = EIO;
+                    default:
+                        errno = EIO;
+                }
+                return -1;
             }
-            return -1;
         }
-
         return 0;
     } CATCH_AND_REPORT()
 
@@ -708,11 +703,9 @@ int redis_fsync_impl(int fd) {
 int redis_fstat_impl(int fd, struct __stat64 *buffer) {
     try {
         int posixFD = RFDMap::getInstance().lookupPosixFD( fd );
-        if( posixFD == -1 ) {
-            posixFD = fd;
+        if (posixFD != -1) {
+            return _fstat64(posixFD, buffer);
         }
-
-        return _fstat64(posixFD, buffer);
     } CATCH_AND_REPORT()
 
     errno = EBADF;
@@ -740,26 +733,20 @@ int redis_listen_impl(int sockfd, int backlog) {
 int redis_ftruncate_impl(int fd, PORT_LONGLONG length) {
     try
     {
-        LARGE_INTEGER l, o;
-        HANDLE h = INVALID_HANDLE_VALUE;
-
         int posixFD = RFDMap::getInstance().lookupPosixFD( fd );
-        if( posixFD == -1 ) {
-            h = (HANDLE) crtget_osfhandle (fd);
-        } else {
-            h = (HANDLE) crtget_osfhandle (posixFD);
-        }
-
-        if( h == INVALID_HANDLE_VALUE) {
-            errno = EBADF;
-            return -1;
-        }
-
-        l.QuadPart = length;
+        if (posixFD != -1) {
+            HANDLE h = (HANDLE) crtget_osfhandle(posixFD);
+            if (h == INVALID_HANDLE_VALUE) {
+                errno = EBADF;
+                return -1;
+            }
 
-        if (!SetFilePointerEx(h, l, &o, FILE_BEGIN)) return -1;
-        if (!SetEndOfFile(h)) return -1;
+            LARGE_INTEGER l, o;
+            l.QuadPart = length;
 
+            if (!SetFilePointerEx(h, l, &o, FILE_BEGIN)) return -1;
+            if (!SetEndOfFile(h)) return -1;
+        }
         return 0;
     } CATCH_AND_REPORT();
 
@@ -986,11 +973,29 @@ u_short redis_ntohs_impl(u_short netshort) {
 }
 
 int redis_setmode_impl(int fd,int mode) {
-    return crtsetmode(fd,mode);
+    return crt_setmode(fd,mode);
 }
 
 size_t redis_fwrite_impl(const void * _Str, size_t _Size, size_t _Count, FILE * _File) {
-    return crtfwrite(_Str, _Size, _Count, _File);
+    return crt_fwrite(_Str, _Size, _Count, _File);
+}
+
+int redis_fclose_impl(FILE * file) {
+    int posixFD = crt_fileno(file);
+    if (posixFD != -1) {
+        RFDMap::getInstance().removePosixFD(posixFD);
+    }
+    return crt_fclose(file);
+}
+
+int redis_fileno_impl(FILE* file) {
+    int rfd = -1;
+    int posixFD = crt_fileno(file);
+    if (posixFD != -1) {
+        // if posixFD is already mapped, addPosixFD() will return the existing rfd.
+        rfd = RFDMap::getInstance().addPosixFD(posixFD);
+    }
+    return rfd;
 }
 
 auto f_select = dllfunctor_stdcall<int, int, fd_set*, fd_set*, fd_set*, const struct timeval*>("ws2_32.dll", "select");
@@ -1048,11 +1053,8 @@ int redis_access_impl(const char *pathname, int mode) {
 u_int64 redis_lseek64_impl(int fd, u_int64 offset, int whence) {
    try {
         int posixFD = RFDMap::getInstance().lookupPosixFD(fd);
-        if( posixFD != -1) {
+        if (posixFD != -1) {
             return crt_lseek64(posixFD, offset, whence);
-        } else {
-            errno = EBADF;
-            return 0;
         }
     } CATCH_AND_REPORT();
 
@@ -1063,11 +1065,8 @@ u_int64 redis_lseek64_impl(int fd, u_int64 offset, int whence) {
 intptr_t redis_get_osfhandle_impl(int fd) {
    try {
         int posixFD = RFDMap::getInstance().lookupPosixFD(fd);
-        if( posixFD != -1) {
+        if (posixFD != -1) {
             return crtget_osfhandle(posixFD);
-        } else {
-            errno = EBADF;
-            return 0;
         }
     } CATCH_AND_REPORT();
 
@@ -1191,6 +1190,8 @@ class Win32_FDSockMap {
         gethostbyname = redis_gethostbyname_impl;
         inet_ntoa = redis_inet_ntoa_impl; 
         fdapi_fwrite = redis_fwrite_impl;
+        fdapi_fclose = redis_fclose_impl;
+        fdapi_fileno = redis_fileno_impl;
         fdapi_setmode = redis_setmode_impl;
         WSASetLastError = redis_WSASetLastError_impl;
         WSAGetLastError = redis_WSAGetLastError_impl;
diff --git a/src/Win32_Interop/Win32_FDAPI.h b/src/Win32_Interop/Win32_FDAPI.h
@@ -152,6 +152,8 @@ typedef int (*redis_WSASocket)(int af, int type, int protocol, LPWSAPROTOCOL_INF
 // other API forwards
 typedef int (*redis_setmode)(int fd,int mode);
 typedef size_t (*redis_fwrite)(const void * _Str, size_t _Size, size_t _Count, FILE * _File);
+typedef int (*redis_fclose)(FILE* file);
+typedef int (*redis_fileno)(FILE* file);
 
 // API prototypes must match the unix implementation
 typedef int (*redis_pipe)(int pipefd[2]);
@@ -237,6 +239,8 @@ extern redis_getsockname getsockname;
 extern redis_ntohs ntohs;
 extern redis_setmode fdapi_setmode;
 extern redis_fwrite fdapi_fwrite;
+extern redis_fclose fdapi_fclose;
+extern redis_fileno fdapi_fileno;
 
 extern redis_select select;
 extern redis_ntohl ntohl;
@@ -267,6 +271,8 @@ int StorageSize(const SOCKADDR_STORAGE *ss);
 #define close(fd) fdapi_close(fd)
 #define setmode(fd,mode) fdapi_setmode(fd,mode)
 #define fwrite(Str, Size, Count, File) fdapi_fwrite(Str,Size,Count,File)
+#define fclose(File) fdapi_fclose(File)
+#define fileno(File) fdapi_fileno(File)
 #define _get_osfhandle(fd) fdapi_get_osfhandle(fd)
 
 #define _INC_STAT_INL
diff --git a/src/Win32_Interop/Win32_fdapi_crt.cpp b/src/Win32_Interop/Win32_fdapi_crt.cpp
@@ -52,15 +52,15 @@ intptr_t crtget_osfhandle(int fd) {
 	return _get_osfhandle(fd);
 }
 
-int crtsetmode(int fd, int mode) {
+int crt_setmode(int fd, int mode) {
 	return ::_setmode(fd, mode);
 }
 
-size_t crtfwrite(const void * _Str, size_t _Size, size_t _Count, FILE * _File) {
+size_t crt_fwrite(const void * _Str, size_t _Size, size_t _Count, FILE * _File) {
     // fwrite() somehow locks its view of the buffer. If during a fork operation the buffer has not been loaded into the forkee's process space,
     // the VEH will be called to load the missing pages. Although the page gets loaded, fwrite() will not see the loaded page. The result is
     // that fwrite will fail with errno set to ERROR_INVALID_USER_BUFFER. The fix is to force the buffer into memory before fwrite(). This only
-    // impacts wirites that straddle page boundaries.
+    // impacts writes that straddle page boundaries.
     const intptr_t pageSize = 4096;
     char* p = (char*)_Str;
     char* pageStart = p - ((intptr_t)p % pageSize);
@@ -74,6 +74,13 @@ size_t crtfwrite(const void * _Str, size_t _Size, size_t _Count, FILE * _File) {
     return ::fwrite(_Str, _Size, _Count, _File);
 }
 
+int crt_fclose(FILE* file) {
+    return ::fclose(file);
+}
+
+int crt_fileno(FILE* file) {
+    return ::_fileno(file);
+}
 
 int crt_isatty(int fd) {
 	return _isatty(fd);
diff --git a/src/Win32_Interop/Win32_fdapi_crt.h b/src/Win32_Interop/Win32_fdapi_crt.h
@@ -31,8 +31,10 @@ int crt_write(int fd, const void *buffer, unsigned int count);
 int crt_open(const char *filename, int oflag, int pmode);
 int crt_open_osfhandle(intptr_t osfhandle, int flags);
 intptr_t crtget_osfhandle(int fd);
-int crtsetmode(int fd, int mode);
-size_t crtfwrite(const void * _Str, size_t _Size, size_t _Count, FILE * _File);
+int crt_setmode(int fd, int mode);
+size_t crt_fwrite(const void * _Str, size_t _Size, size_t _Count, FILE * _File);
+int crt_fclose(FILE* file);
+int crt_fileno(FILE* file);
 int crt_isatty(int fd);
 int crt_access(const char *pathname, int mode);
 __int64 crt_lseek64(int fd, __int64 offset, int origin);
diff --git a/src/Win32_Interop/win32_rfdmap.cpp b/src/Win32_Interop/win32_rfdmap.cpp
@@ -31,7 +31,12 @@ RFDMap& RFDMap::getInstance() {
 
 RFDMap::RFDMap() {
     InitializeCriticalSection(&mutex);
-    maxRFD = minRFD;
+    // stdin, assigned rfd = 0
+    addPosixFD(0);
+    // stdout, assigned rfd = 1
+    addPosixFD(1);
+    // stderr, assigned rfd = 2
+    addPosixFD(2);
 }
 
 RFD RFDMap::getNextRFDAvailable() {
@@ -41,8 +46,7 @@ RFD RFDMap::getNextRFDAvailable() {
         rfd = RFDRecyclePool.front();
         RFDRecyclePool.pop();
     } else {
-        maxRFD = minRFD + (int) SocketToRFDMap.size() + (int) PosixFDToRFDMap.size();
-        rfd = maxRFD;
+        rfd = (int) SocketToRFDMap.size() + (int) PosixFDToRFDMap.size();
     }
     LeaveCriticalSection(&mutex);
     return rfd;
@@ -78,7 +82,7 @@ RFD RFDMap::addPosixFD(int posixFD) {
     RFD rfd;
     EnterCriticalSection(&mutex);
     if (PosixFDToRFDMap.find(posixFD) != PosixFDToRFDMap.end()) {
-        rfd = invalidRFD;
+        rfd = PosixFDToRFDMap[posixFD];
     } else {
         rfd = getNextRFDAvailable();
         PosixFDToRFDMap[posixFD] = rfd;
@@ -89,15 +93,19 @@ RFD RFDMap::addPosixFD(int posixFD) {
 }
 
 void RFDMap::removePosixFD(int posixFD) {
-    EnterCriticalSection(&mutex);
-    PosixFD2RFDIterator mit = PosixFDToRFDMap.find(posixFD);
-    if (mit != PosixFDToRFDMap.end()) {
-        RFD rfd = (*mit).second;
-        RFDRecyclePool.push(rfd);
-        RFDToPosixFDMap.erase(rfd);
-        PosixFDToRFDMap.erase(posixFD);
+    // posixFD between 0 and 2 should never be removed since they are assigned
+    // to stdin, stdout and stderr
+    if (posixFD > 2) {
+        EnterCriticalSection(&mutex);
+        PosixFD2RFDIterator mit = PosixFDToRFDMap.find(posixFD);
+        if (mit != PosixFDToRFDMap.end()) {
+            RFD rfd = (*mit).second;
+            RFDRecyclePool.push(rfd);
+            RFDToPosixFDMap.erase(rfd);
+            PosixFDToRFDMap.erase(posixFD);
+        }
+        LeaveCriticalSection(&mutex);
     }
-    LeaveCriticalSection(&mutex);
 }
 
 SOCKET RFDMap::lookupSocket(RFD rfd) {
diff --git a/src/Win32_Interop/win32_rfdmap.h b/src/Win32_Interop/win32_rfdmap.h
@@ -78,8 +78,6 @@ class RFDMap {
     RFDRecyclePoolType RFDRecyclePool;
 
 private:
-    const static int minRFD = 3;    // 0, 1 and 2 are reserved for stdin, stdout and stderr
-    RFD maxRFD;
     CRITICAL_SECTION mutex;
 
 public:
