msvcrt: Added _fread_nolock_s implementation (try 2)

Iván Matellanes matellanesivan at gmail.com
Sun Nov 9 13:17:45 CST 2014


Hi,
I've checked when the file gets locked after calling fread_s.
It turns out that the locking is done after validating the stream, the element size and
the element count. This potentially leads to calling the invalid parameter handler with
the file locked if, for example, destBuffer == NULL (at least that's what happens on my
Windows 7 x64 machine).
This implementation tries to reproduce this behaviour.
Cheers!

---
 dlls/msvcr100/msvcr100.spec    |  2 +-
 dlls/msvcr100/tests/msvcr100.c | 35 +++++++++++++++++++++++++++++++++++
 dlls/msvcr110/msvcr110.spec    |  2 +-
 dlls/msvcr120/msvcr120.spec    |  2 +-
 dlls/msvcr80/msvcr80.spec      |  2 +-
 dlls/msvcr90/msvcr90.spec      |  2 +-
 dlls/msvcrt/file.c             | 26 ++++++++++++++++++++++++--
 dlls/msvcrt/msvcrt.h           |  1 +
 include/msvcrt/stdio.h         |  2 ++
 9 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/dlls/msvcr100/msvcr100.spec b/dlls/msvcr100/msvcr100.spec
index cb45506..a6e7771 100644
--- a/dlls/msvcr100/msvcr100.spec
+++ b/dlls/msvcr100/msvcr100.spec
@@ -835,7 +835,7 @@
 @ cdecl _fputwc_nolock(long ptr) MSVCRT__fputwc_nolock
 @ cdecl _fputwchar(long) MSVCRT__fputwchar
 @ cdecl _fread_nolock(ptr long long ptr) MSVCRT__fread_nolock
-@ stub _fread_nolock_s
+@ cdecl _fread_nolock_s(ptr long long long ptr) MSVCRT__fread_nolock_s
 @ cdecl _free_locale(ptr) MSVCRT__free_locale
 @ stub _freea
 @ stub _freea_s
diff --git a/dlls/msvcr100/tests/msvcr100.c b/dlls/msvcr100/tests/msvcr100.c
index a998fb5..4c66ed5 100644
--- a/dlls/msvcr100/tests/msvcr100.c
+++ b/dlls/msvcr100/tests/msvcr100.c
@@ -140,6 +140,8 @@ static int (__cdecl *p_wmemmove_s)(wchar_t *dest, size_t numberOfElements, const
 static FILE* (__cdecl *p_fopen)(const char*,const char*);
 static int (__cdecl *p_fclose)(FILE*);
 static size_t (__cdecl *p_fread_s)(void*,size_t,size_t,size_t,FILE*);
+static void (__cdecl *p_lock_file)(FILE*);
+static void (__cdecl *p_unlock_file)(FILE*);
 static void* (__cdecl *p__aligned_offset_malloc)(size_t, size_t, size_t);
 static void (__cdecl *p__aligned_free)(void*);
 static size_t (__cdecl *p__aligned_msize)(void*, size_t, size_t);
@@ -178,6 +180,8 @@ static BOOL init(void)
     SET(p_fopen, "fopen");
     SET(p_fclose, "fclose");
     SET(p_fread_s, "fread_s");
+    SET(p_lock_file, "_lock_file");
+    SET(p_unlock_file, "_unlock_file");
     SET(p__aligned_offset_malloc, "_aligned_offset_malloc");
     SET(p__aligned_free, "_aligned_free");
     SET(p__aligned_msize, "_aligned_msize");
@@ -357,11 +361,30 @@ static void test_wmemmove_s(void)
             "Cannot reset invalid parameter handler\n");
 }
 
+struct block_file_arg
+{
+    FILE *test_file;
+    HANDLE init;
+    HANDLE finish;
+};
+
+static DWORD WINAPI block_file(void *arg)
+{
+    struct block_file_arg *files = arg;
+    p_lock_file(files->test_file);
+    SetEvent(files->init);
+    WaitForSingleObject(files->finish, INFINITE);
+    p_unlock_file(files->test_file);
+    return 0;
+}
+
 static void test_fread_s(void)
 {
     static const char test_file[] = "fread_s.tst";
     int ret;
     char buf[10];
+    HANDLE thread;
+    struct block_file_arg arg;
 
     FILE *f = fopen(test_file, "w");
     if(!f) {
@@ -382,6 +405,12 @@ static void test_fread_s(void)
     CHECK_CALLED(invalid_parameter_handler);
 
     f = p_fopen(test_file, "r");
+    arg.test_file = f;
+    arg.init = CreateEventW(NULL, FALSE, FALSE, NULL);
+    arg.finish = CreateEventW(NULL, FALSE, FALSE, NULL);
+    thread = CreateThread(NULL, 0, block_file, (void*)&arg, 0, NULL);
+    WaitForSingleObject(arg.init, INFINITE);
+
     errno = 0xdeadbeef;
     ret = p_fread_s(NULL, sizeof(buf), 0, 1, f);
     ok(ret == 0, "fread_s returned %d, expected 0\n", ret);
@@ -390,6 +419,9 @@ static void test_fread_s(void)
     ok(ret == 0, "fread_s returned %d, expected 0\n", ret);
     ok(errno == 0xdeadbeef, "errno = %d, expected 0xdeadbeef\n", errno);
 
+    SetEvent(arg.finish);
+    WaitForSingleObject(thread, INFINITE);
+
     SET_EXPECT(invalid_parameter_handler);
     errno = 0xdeadbeef;
     ret = p_fread_s(NULL, sizeof(buf), 1, 1, f);
@@ -426,6 +458,9 @@ static void test_fread_s(void)
 
     ok(p_set_invalid_parameter_handler(NULL) == test_invalid_parameter_handler,
             "Cannot reset invalid parameter handler\n");
+    CloseHandle(arg.init);
+    CloseHandle(arg.finish);
+    CloseHandle(thread);
     unlink(test_file);
 }
 
diff --git a/dlls/msvcr110/msvcr110.spec b/dlls/msvcr110/msvcr110.spec
index d41699d..141697c 100644
--- a/dlls/msvcr110/msvcr110.spec
+++ b/dlls/msvcr110/msvcr110.spec
@@ -1183,7 +1183,7 @@
 @ cdecl _fputwc_nolock(long ptr) MSVCRT__fputwc_nolock
 @ cdecl _fputwchar(long) MSVCRT__fputwchar
 @ cdecl _fread_nolock(ptr long long ptr) MSVCRT__fread_nolock
-@ stub _fread_nolock_s
+@ cdecl _fread_nolock_s(ptr long long long ptr) MSVCRT__fread_nolock_s
 @ cdecl _free_locale(ptr) MSVCRT__free_locale
 @ stub _freea
 @ stub _freea_s
diff --git a/dlls/msvcr120/msvcr120.spec b/dlls/msvcr120/msvcr120.spec
index fbf688e..1b8dc28 100644
--- a/dlls/msvcr120/msvcr120.spec
+++ b/dlls/msvcr120/msvcr120.spec
@@ -1181,7 +1181,7 @@
 @ cdecl _fputwc_nolock(long ptr) MSVCRT__fputwc_nolock
 @ cdecl _fputwchar(long) MSVCRT__fputwchar
 @ cdecl _fread_nolock(ptr long long ptr) MSVCRT__fread_nolock
-@ stub _fread_nolock_s
+@ cdecl _fread_nolock_s(ptr long long long ptr) MSVCRT__fread_nolock_s
 @ cdecl _free_locale(ptr) MSVCRT__free_locale
 @ stub _freea
 @ stub _freea_s
diff --git a/dlls/msvcr80/msvcr80.spec b/dlls/msvcr80/msvcr80.spec
index efb4539..fcad7cd 100644
--- a/dlls/msvcr80/msvcr80.spec
+++ b/dlls/msvcr80/msvcr80.spec
@@ -502,7 +502,7 @@
 @ cdecl _fputwc_nolock(long ptr) MSVCRT__fputwc_nolock
 @ cdecl _fputwchar(long) MSVCRT__fputwchar
 @ cdecl _fread_nolock(ptr long long ptr) MSVCRT__fread_nolock
-@ stub _fread_nolock_s
+@ cdecl _fread_nolock_s(ptr long long long ptr) MSVCRT__fread_nolock_s
 @ cdecl _free_locale(ptr) MSVCRT__free_locale
 @ stub _freea
 @ stub _freea_s
diff --git a/dlls/msvcr90/msvcr90.spec b/dlls/msvcr90/msvcr90.spec
index fa62366..7328b46 100644
--- a/dlls/msvcr90/msvcr90.spec
+++ b/dlls/msvcr90/msvcr90.spec
@@ -484,7 +484,7 @@
 @ cdecl _fputwc_nolock(long ptr) MSVCRT__fputwc_nolock
 @ cdecl _fputwchar(long) MSVCRT__fputwchar
 @ cdecl _fread_nolock(ptr long long ptr) MSVCRT__fread_nolock
-@ stub _fread_nolock_s
+@ cdecl _fread_nolock_s(ptr long long long ptr) MSVCRT__fread_nolock_s
 @ cdecl _free_locale(ptr) MSVCRT__free_locale
 @ stub _freea
 @ stub _freea_s
diff --git a/dlls/msvcrt/file.c b/dlls/msvcrt/file.c
index 491942d..5049605 100644
--- a/dlls/msvcrt/file.c
+++ b/dlls/msvcrt/file.c
@@ -4130,9 +4130,31 @@ MSVCRT_size_t CDECL MSVCRT__fread_nolock(void *ptr, MSVCRT_size_t size, MSVCRT_s
 MSVCRT_size_t CDECL MSVCRT_fread_s(void *buf, MSVCRT_size_t buf_size, MSVCRT_size_t elem_size,
         MSVCRT_size_t count, MSVCRT_FILE *stream)
 {
+    MSVCRT_size_t ret;
+
+    if(!MSVCRT_CHECK_PMT(stream != NULL)) {
+        if(buf && buf_size)
+            memset(buf, 0, buf_size);
+        return 0;
+    }
+    if(!elem_size || !count) return 0;
+
+    MSVCRT__lock_file(stream);
+    ret = MSVCRT__fread_nolock_s(buf, buf_size, elem_size, count, stream);
+    MSVCRT__unlock_file(stream);
+
+    return ret;
+}
+
+/*********************************************************************
+ *		_fread_nolock_s (MSVCR80.@)
+ */
+MSVCRT_size_t CDECL MSVCRT__fread_nolock_s(void *buf, MSVCRT_size_t buf_size, MSVCRT_size_t elem_size,
+        MSVCRT_size_t count, MSVCRT_FILE *stream)
+{
     size_t bytes_left, buf_pos;
 
-    TRACE("(%p %lu %lu %lu %p\n", buf, buf_size, elem_size, count, stream);
+    TRACE("(%p %lu %lu %lu %p)\n", buf, buf_size, elem_size, count, stream);
 
     if(!MSVCRT_CHECK_PMT(stream != NULL)) {
         if(buf && buf_size)
@@ -4154,7 +4176,7 @@ MSVCRT_size_t CDECL MSVCRT_fread_s(void *buf, MSVCRT_size_t buf_size, MSVCRT_siz
                 return 0;
             }
 
-            MSVCRT_fread((char*)buf+buf_pos, 1, size, stream);
+            MSVCRT__fread_nolock((char*)buf+buf_pos, 1, size, stream);
             buf_pos += size;
             bytes_left -= size;
         }else {
diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h
index 70ac68b..72d529d 100644
--- a/dlls/msvcrt/msvcrt.h
+++ b/dlls/msvcrt/msvcrt.h
@@ -934,6 +934,7 @@ MSVCRT_ulong* __cdecl MSVCRT___doserrno(void);
 int* __cdecl     MSVCRT__errno(void);
 char* __cdecl    MSVCRT_getenv(const char*);
 MSVCRT_size_t __cdecl MSVCRT__fread_nolock(void*,MSVCRT_size_t,MSVCRT_size_t,MSVCRT_FILE*);
+MSVCRT_size_t __cdecl MSVCRT__fread_nolock_s(void*,MSVCRT_size_t,MSVCRT_size_t,MSVCRT_size_t,MSVCRT_FILE*);
 MSVCRT_size_t __cdecl MSVCRT__fwrite_nolock(const void*,MSVCRT_size_t,MSVCRT_size_t,MSVCRT_FILE*);
 int __cdecl      MSVCRT_fclose(MSVCRT_FILE*);
 int __cdecl      MSVCRT__fclose_nolock(MSVCRT_FILE*);
diff --git a/include/msvcrt/stdio.h b/include/msvcrt/stdio.h
index eda89e6..0606d07 100644
--- a/include/msvcrt/stdio.h
+++ b/include/msvcrt/stdio.h
@@ -128,6 +128,7 @@ int    __cdecl _vsnprintf_s(char*,size_t,size_t,const char*,__ms_va_list);
 int    __cdecl _vsprintf_p_l(char*,size_t,const char*,_locale_t,__ms_va_list);
 
 size_t __cdecl _fread_nolock(void*,size_t,size_t,FILE*);
+size_t __cdecl _fread_nolock_s(void*,size_t,size_t,size_t,FILE*);
 size_t __cdecl _fwrite_nolock(const void*,size_t,size_t,FILE*);
 int    __cdecl _fclose_nolock(FILE*);
 int    __cdecl _fflush_nolock(FILE*);
@@ -156,6 +157,7 @@ int    __cdecl fprintf_s(FILE*,const char*,...);
 int    __cdecl fputc(int,FILE*);
 int    __cdecl fputs(const char*,FILE*);
 size_t __cdecl fread(void*,size_t,size_t,FILE*);
+size_t __cdecl fread_s(void*,size_t,size_t,size_t,FILE*);
 FILE*  __cdecl freopen(const char*,const char*,FILE*);
 int    __cdecl fscanf(FILE*,const char*,...);
 int    __cdecl fscanf_s(FILE*,const char*,...);
-- 
1.9.1



More information about the wine-patches mailing list