[PATCH v3 0/2] MR3578: freopen() fix.

[eric pouech (＠epo)]

I stumbled onto this while using freopen() for debugging purposes.
Basically, freopen() fails if the FILE has been created with
an invalid handle.

So, this MR contains:
- basic tests for freopen (no issue there, just for coverage purposes)
- tests for freopen on FILE with invalid handle
- fix for freopen

--
  v3: msvcrt: Fix freopen() on FILE with invalid underlying fd.
      msvcrt/tests: Add tests for freopen().

https://gitlab.winehq.org/wine/wine/-/merge_requests/3578

[PATCH v3 1/2] msvcrt/tests: Add tests for freopen().

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 dlls/msvcrt/tests/file.c | 74 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/dlls/msvcrt/tests/file.c b/dlls/msvcrt/tests/file.c
index 3f57b3f871f..51ba4225032 100644
--- a/dlls/msvcrt/tests/file.c
+++ b/dlls/msvcrt/tests/file.c
@@ -1167,6 +1167,79 @@ static void test_fputwc(void)
     _unlink(tempfile);
 }

+static void test_freopen( void )
+{
+    char filename1[8] = "AXXXXXX";
+    char filename2[8] = "BXXXXXX";
+    FILE *file;
+    FILE *new;
+    int ret;
+    int fd;
+    char ch;
+    long pos;
+
+    mktemp(filename1);
+    mktemp(filename2);
+
+    file = fopen(filename1, "wt");
+    ok(file != NULL, "fopen(filename1) returned NULL\n");
+    ret = fwrite("1", 1, 1, file);
+    ok(ret == 1, "fwrite() returned %d (%d)\n", ret, errno);
+    ret = fclose(file);
+    ok(ret == 0, "fclose() returned %d\n", ret);
+
+    file = fopen(filename2, "wt");
+    ok(file != NULL, "fopen(filename1) returned NULL\n");
+    ret = fwrite("2", 1, 1, file);
+    ok(ret == 1, "fwrite() returned %d (%d)\n", ret, errno);
+    ret = fclose(file);
+    ok(ret == 0, "fclose() returned %d\n", ret);
+
+    file = fopen(filename1, "rt");
+    ok(file != NULL, "fopen(filename1) returned NULL\n");
+    file = freopen(filename2, "rt", file);
+    ok(file != NULL, "fopen(filename2) returned NULL\n");
+    ch = '#';
+    ret = fread(&ch, 1, 1, file);
+    ok(ret == 1, "fread() returned %d\n", ret);
+    ok(ch == '2', "fread() read %c\n", ch);
+    ret = fclose(file);
+    ok(ret == 0, "fclose() returned %d\n", ret);
+
+    file = fopen(filename1, "at");
+    ok(file != NULL, "fopen(filename1) returned NULL\n");
+    file = freopen(filename1, "rt", file);
+    ok(file != NULL, "fopen(filename1) returned NULL\n");
+    pos = ftell(file);
+    ok(pos == 0, "ftell() returned %ld\n", pos);
+    ch = '#';
+    ret = fread(&ch, 1, 1, file);
+    ok(ret == 1, "fread() returned %d\n", ret);
+    ok(ch == '1', "fread() read %c\n", ch);
+    ret = fclose(file);
+    ok(ret == 0, "fclose() returned %d\n", ret);
+
+    file = fopen(filename1, "rt");
+    ok(file != NULL, "fopen(filename1) returned NULL\n");
+    fd = fileno(file);
+    ok(fd > 0, "fileno() returned %d\n", fd);
+    /* invalid filename */
+    new = freopen("_:", "rt", file);
+    ok(new == NULL, "fopen(_:) returned non NULL\n");
+    errno = 0xdeadbeef;
+    ch = '#';
+    ret = read(fd, &ch, 1);
+    ok(ret == -1, "read() returned %d\n", ret);
+    ok(errno == EBADF, "errno is %d\n", errno);
+    errno = 0xdeadbeef;
+    ret = fclose(file);
+    ok(ret == EOF, "fclose(file) succeeded\n");
+    ok(errno == 0xdeadbeef, "errno is %d\n", errno);
+
+    unlink(filename1);
+    unlink(filename2);
+}
+
 static void test_ctrlz( void )
 {
   char* tempf;
@@ -2977,6 +3050,7 @@ START_TEST(file)
     test_fgetwc_locale("AB\x83\xa9", "C", 0);
     test_fgetwc_unicode();
     test_fputwc();
+    test_freopen();
     test_ctrlz();
     test_file_put_get();
     test_tmpnam();

[PATCH v3 2/2] msvcrt: Fix freopen() on FILE with invalid underlying fd.

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 dlls/msvcrt/file.c       |  4 +---
 dlls/msvcrt/tests/file.c | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/dlls/msvcrt/file.c b/dlls/msvcrt/file.c
index e72784eef41..24d801a5306 100644
--- a/dlls/msvcrt/file.c
+++ b/dlls/msvcrt/file.c
@@ -4588,9 +4588,7 @@ FILE* CDECL _wfreopen(const wchar_t *path, const wchar_t *mode, FILE* file)
     TRACE(":path (%s) mode (%s) file (%p) fd (%d)\n", debugstr_w(path), debugstr_w(mode), file, file ? file->_file : -1);

LOCK_FILES();
-    if (!file || ((fd = file->_file) < 0))
-        file = NULL;
-    else
+    if (file)
     {
         fclose(file);
         if (msvcrt_get_flags(mode, &open_flags, &stream_flags) == -1)
diff --git a/dlls/msvcrt/tests/file.c b/dlls/msvcrt/tests/file.c
index 51ba4225032..15f18a395c8 100644
--- a/dlls/msvcrt/tests/file.c
+++ b/dlls/msvcrt/tests/file.c
@@ -1236,6 +1236,25 @@ static void test_freopen( void )
     ok(ret == EOF, "fclose(file) succeeded\n");
     ok(errno == 0xdeadbeef, "errno is %d\n", errno);

+    file = fopen(filename1, "rb");
+    ok(file != NULL, "couldn't open %s\n", filename1);
+    close(file->_file);
+    file->_file = -1;
+
+    new = freopen(filename2, "rb", file);
+    ok(new == file, "freopen() didn't return same FILE*\n");
+
+    fd = fileno(file);
+    ok(fd > 0, "fileno() returned %d\n", fd);
+
+    ch = '#';
+    ret = fread(&ch, 1, 1, file);
+    ok(ret == 1, "fread() returned %d\n", ret);
+    ok(ch == '2', "Unexpected char\n");
+
+    ret = fclose(file);
+    ok(ret == 0, "fclose(file) returned %d\n", ret);
+
     unlink(filename1);
     unlink(filename2);
 }

[eric pouech (＠epo)]

On Thu Aug 17 08:16:49 2023 +0000, eric pouech wrote:
> changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/3578/diffs?diff_id=63129&start_sha=a4ab6dc29a06b796f6a778ba93a5d97bda5487fa#1a28d67ae45bfb1e18198f92ca618b484c4e5a63_1256_1255)

V3 pushed:
- moved the invalid file bits of patch #3 into #1 (no need to test this twice...), including fclose()==EOF suggestion
- merged patches #2 and #3 to avoid convolutions
- minor cleanups

[PATCH v3 0/2] MR3578: freopen() fix. - approved

[Piotr Caban (＠piotr)]

This merge request was approved by Piotr Caban.