Signed-off-by: Zebediah Figura z.figura12@gmail.com --- v2: don't assume cwd == executable directory
dlls/winmm/tests/mmio.c | 60 +++++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 8 deletions(-)
diff --git a/dlls/winmm/tests/mmio.c b/dlls/winmm/tests/mmio.c index e3984fbaf3a..0236855f246 100644 --- a/dlls/winmm/tests/mmio.c +++ b/dlls/winmm/tests/mmio.c @@ -478,23 +478,34 @@ static void test_mmioOpen_create(void) WCHAR cwd[MAX_PATH], temp_dir[MAX_PATH]; /* According to docs, filename must be no more than 128 bytes, but it will * actually allow longer than that. */ - WCHAR filename[] = L"very_long_filename_" + WCHAR long_filename[] = L"very_long_filename_" L"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" L"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" L"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"; + WCHAR buffer[MAX_PATH], expect[MAX_PATH]; + char exedir_filename[MAX_PATH]; + MMIOINFO info = {0}; + BOOL ret; + FILE *f; + + GetModuleFileNameA(NULL, exedir_filename, ARRAY_SIZE(exedir_filename)); + strcpy(strrchr(exedir_filename, '\') + 1, "test_mmio_path"); + f = fopen(exedir_filename, "w"); + ok(!!f, "failed to create %s: %s\n", debugstr_a(exedir_filename), strerror(errno)); + fclose(f);
GetCurrentDirectoryW(ARRAY_SIZE(cwd), cwd); GetTempPathW(ARRAY_SIZE(temp_dir), temp_dir); SetCurrentDirectoryW(temp_dir);
- DeleteFileW(filename); + DeleteFileW(long_filename);
/* open with MMIO_DENYNONE */ - hmmio = mmioOpenW(filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_DENYNONE); + hmmio = mmioOpenW(long_filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_DENYNONE); ok(hmmio != NULL, "mmioOpen failed\n");
/* MMIO_DENYNONE lets us open it here, too */ - handle = CreateFileW(filename, GENERIC_READ, + handle = CreateFileW(long_filename, GENERIC_READ, FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); ok(handle != INVALID_HANDLE_VALUE, "Couldn't open non-exclusive file\n"); @@ -502,14 +513,14 @@ static void test_mmioOpen_create(void)
mmioClose(hmmio, 0);
- DeleteFileW(filename); + DeleteFileW(long_filename);
/* open with MMIO_EXCLUSIVE */ - hmmio = mmioOpenW(filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_EXCLUSIVE); + hmmio = mmioOpenW(long_filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_EXCLUSIVE); ok(hmmio != NULL, "mmioOpen failed\n");
/* should fail due to MMIO_EXCLUSIVE */ - handle = CreateFileW(filename, GENERIC_READ, + handle = CreateFileW(long_filename, GENERIC_READ, FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); ok(handle == INVALID_HANDLE_VALUE, "Opening exclusive file should have failed\n"); @@ -518,9 +529,42 @@ static void test_mmioOpen_create(void)
mmioClose(hmmio, 0);
- DeleteFileW(filename); + DeleteFileW(long_filename); + + wcscpy(buffer, L"test_mmio_path"); + hmmio = mmioOpenW(buffer, &info, MMIO_WRITE); + todo_wine ok(!!hmmio, "failed to open file, error %#x\n", info.wErrorRet); + mmioClose(hmmio, 0); + + wcscpy(buffer, L"test_mmio_path"); + hmmio = mmioOpenW(buffer, &info, MMIO_PARSE); + ok(hmmio == (HMMIO)TRUE, "failed to parse file name, error %#x\n", info.wErrorRet); + wcscpy(expect, temp_dir); + wcscat(expect, L"test_mmio_path"); + todo_wine ok(!wcscmp(buffer, expect), "expected %s, got %s\n", debugstr_w(expect), debugstr_w(buffer)); + + wcscpy(buffer, L"test_mmio_path"); + info.wErrorRet = 0xdead; + hmmio = mmioOpenW(buffer, &info, MMIO_EXIST); + ok(hmmio == (HMMIO)FALSE, "file should exist\n"); + todo_wine ok(info.wErrorRet == MMIOERR_FILENOTFOUND, "got error %#x\n", info.wErrorRet); + + ret = DeleteFileA("test_mmio_path"); + ok(!ret, "expected failure\n"); + ok(GetLastError() == ERROR_FILE_NOT_FOUND, "got error %u\n", GetLastError()); + + wcscpy(buffer, L"test_mmio_path"); + hmmio = mmioOpenW(buffer, &info, MMIO_WRITE | MMIO_CREATE); + ok(!!hmmio, "failed to open file, error %#x\n", info.wErrorRet); + mmioClose(hmmio, 0); + + ret = DeleteFileA("test_mmio_path"); + ok(ret, "got error %u\n", GetLastError());
SetCurrentDirectoryW(cwd); + + ret = DeleteFileA(exedir_filename); + ok(ret, "got error %u\n", GetLastError()); }
static void test_mmioSetBuffer(char *fname)
Based on a patch by Alistair Leslie-Hughes.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49650 Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/winmm/mmio.c | 24 ++++++++++++++++++++++-- dlls/winmm/tests/mmio.c | 2 +- 2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/dlls/winmm/mmio.c b/dlls/winmm/mmio.c index 48bbd3ed48d..c4838f388b6 100644 --- a/dlls/winmm/mmio.c +++ b/dlls/winmm/mmio.c @@ -51,7 +51,9 @@ static WINE_MMIO *MMIOList; /* From kernel32 */ static HANDLE create_file_OF( LPCSTR path, INT mode ) { - DWORD access, sharing, creation; + DWORD access, sharing, creation, len; + char *full_path; + HANDLE ret;
if (mode & OF_CREATE) { @@ -79,7 +81,25 @@ static HANDLE create_file_OF( LPCSTR path, INT mode ) case OF_SHARE_COMPAT: default: sharing = FILE_SHARE_READ | FILE_SHARE_WRITE; break; } - return CreateFileA( path, access, sharing, NULL, creation, FILE_ATTRIBUTE_NORMAL, 0 ); + + if (mode & OF_CREATE) + return CreateFileA( path, access, sharing, NULL, creation, FILE_ATTRIBUTE_NORMAL, 0 ); + + if (!(len = SearchPathA( NULL, path, NULL, 0, NULL, NULL ))) + return INVALID_HANDLE_VALUE; + if (!(full_path = malloc(len + 1))) + { + SetLastError( ERROR_OUTOFMEMORY ); + return INVALID_HANDLE_VALUE; + } + if (!SearchPathA( NULL, path, NULL, len + 1, full_path, NULL )) + { + free(full_path); + return INVALID_HANDLE_VALUE; + } + ret = CreateFileA( full_path, access, sharing, NULL, creation, FILE_ATTRIBUTE_NORMAL, 0 ); + free(full_path); + return ret; }
/************************************************************************** diff --git a/dlls/winmm/tests/mmio.c b/dlls/winmm/tests/mmio.c index 0236855f246..a2abb6c5f5c 100644 --- a/dlls/winmm/tests/mmio.c +++ b/dlls/winmm/tests/mmio.c @@ -533,7 +533,7 @@ static void test_mmioOpen_create(void)
wcscpy(buffer, L"test_mmio_path"); hmmio = mmioOpenW(buffer, &info, MMIO_WRITE); - todo_wine ok(!!hmmio, "failed to open file, error %#x\n", info.wErrorRet); + ok(!!hmmio, "failed to open file, error %#x\n", info.wErrorRet); mmioClose(hmmio, 0);
wcscpy(buffer, L"test_mmio_path");
Well, you found a new bug. Your tests crash:
https://testbot.winehq.org/JobDetails.pl?Key=83551#k401
This is because in mmioOpenW, we allocate only the space required to fit the input filename, but MMIO_PARSE needs room for at least 128 characters. (128 per the docs. Might as well do MAX_PATH.)
I think you need the below diff before applying your patches. It ought to also copy the resulting filename back to the input buffer, but, code freeze.
diff --git a/dlls/winmm/mmio.c b/dlls/winmm/mmio.c index 48bbd3ed48d..4f13a34303c 100644 --- a/dlls/winmm/mmio.c +++ b/dlls/winmm/mmio.c @@ -733,10 +733,9 @@ HMMIO WINAPI mmioOpenW(LPWSTR szFileName, MMIOINFO* lpmmioinfo,
if (szFileName) { - INT len = WideCharToMultiByte( CP_ACP, 0, szFileName, -1, NULL, 0, NULL, NULL ); - szFn = HeapAlloc( GetProcessHeap(), 0, len ); + szFn = HeapAlloc( GetProcessHeap(), 0, MAX_PATH ); if (!szFn) return NULL; - WideCharToMultiByte( CP_ACP, 0, szFileName, -1, szFn, len, NULL, NULL ); + WideCharToMultiByte( CP_ACP, 0, szFileName, -1, szFn, MAX_PATH, NULL, NULL ); }
ret = MMIO_Open(szFn, lpmmioinfo, dwOpenFlags, TRUE);
Andrew
On Thu, Dec 17, 2020 at 02:34:26PM -0600, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura z.figura12@gmail.com
v2: don't assume cwd == executable directory
dlls/winmm/tests/mmio.c | 60 +++++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 8 deletions(-)
diff --git a/dlls/winmm/tests/mmio.c b/dlls/winmm/tests/mmio.c index e3984fbaf3a..0236855f246 100644 --- a/dlls/winmm/tests/mmio.c +++ b/dlls/winmm/tests/mmio.c @@ -478,23 +478,34 @@ static void test_mmioOpen_create(void) WCHAR cwd[MAX_PATH], temp_dir[MAX_PATH]; /* According to docs, filename must be no more than 128 bytes, but it will * actually allow longer than that. */
- WCHAR filename[] = L"very_long_filename_"
WCHAR long_filename[] = L"very_long_filename_" L"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" L"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" L"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx";
WCHAR buffer[MAX_PATH], expect[MAX_PATH];
char exedir_filename[MAX_PATH];
MMIOINFO info = {0};
BOOL ret;
FILE *f;
GetModuleFileNameA(NULL, exedir_filename, ARRAY_SIZE(exedir_filename));
strcpy(strrchr(exedir_filename, '\') + 1, "test_mmio_path");
f = fopen(exedir_filename, "w");
ok(!!f, "failed to create %s: %s\n", debugstr_a(exedir_filename), strerror(errno));
fclose(f);
GetCurrentDirectoryW(ARRAY_SIZE(cwd), cwd); GetTempPathW(ARRAY_SIZE(temp_dir), temp_dir); SetCurrentDirectoryW(temp_dir);
- DeleteFileW(filename);
DeleteFileW(long_filename);
/* open with MMIO_DENYNONE */
- hmmio = mmioOpenW(filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_DENYNONE);
hmmio = mmioOpenW(long_filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_DENYNONE); ok(hmmio != NULL, "mmioOpen failed\n");
/* MMIO_DENYNONE lets us open it here, too */
- handle = CreateFileW(filename, GENERIC_READ,
- handle = CreateFileW(long_filename, GENERIC_READ, FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); ok(handle != INVALID_HANDLE_VALUE, "Couldn't open non-exclusive file\n");
@@ -502,14 +513,14 @@ static void test_mmioOpen_create(void)
mmioClose(hmmio, 0);
- DeleteFileW(filename);
DeleteFileW(long_filename);
/* open with MMIO_EXCLUSIVE */
- hmmio = mmioOpenW(filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_EXCLUSIVE);
hmmio = mmioOpenW(long_filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_EXCLUSIVE); ok(hmmio != NULL, "mmioOpen failed\n");
/* should fail due to MMIO_EXCLUSIVE */
- handle = CreateFileW(filename, GENERIC_READ,
- handle = CreateFileW(long_filename, GENERIC_READ, FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); ok(handle == INVALID_HANDLE_VALUE, "Opening exclusive file should have failed\n");
@@ -518,9 +529,42 @@ static void test_mmioOpen_create(void)
mmioClose(hmmio, 0);
- DeleteFileW(filename);
DeleteFileW(long_filename);
wcscpy(buffer, L"test_mmio_path");
hmmio = mmioOpenW(buffer, &info, MMIO_WRITE);
todo_wine ok(!!hmmio, "failed to open file, error %#x\n", info.wErrorRet);
mmioClose(hmmio, 0);
wcscpy(buffer, L"test_mmio_path");
hmmio = mmioOpenW(buffer, &info, MMIO_PARSE);
ok(hmmio == (HMMIO)TRUE, "failed to parse file name, error %#x\n", info.wErrorRet);
wcscpy(expect, temp_dir);
wcscat(expect, L"test_mmio_path");
todo_wine ok(!wcscmp(buffer, expect), "expected %s, got %s\n", debugstr_w(expect), debugstr_w(buffer));
wcscpy(buffer, L"test_mmio_path");
info.wErrorRet = 0xdead;
hmmio = mmioOpenW(buffer, &info, MMIO_EXIST);
ok(hmmio == (HMMIO)FALSE, "file should exist\n");
todo_wine ok(info.wErrorRet == MMIOERR_FILENOTFOUND, "got error %#x\n", info.wErrorRet);
ret = DeleteFileA("test_mmio_path");
ok(!ret, "expected failure\n");
ok(GetLastError() == ERROR_FILE_NOT_FOUND, "got error %u\n", GetLastError());
wcscpy(buffer, L"test_mmio_path");
hmmio = mmioOpenW(buffer, &info, MMIO_WRITE | MMIO_CREATE);
ok(!!hmmio, "failed to open file, error %#x\n", info.wErrorRet);
mmioClose(hmmio, 0);
ret = DeleteFileA("test_mmio_path");
ok(ret, "got error %u\n", GetLastError());
SetCurrentDirectoryW(cwd);
ret = DeleteFileA(exedir_filename);
ok(ret, "got error %u\n", GetLastError());
}
static void test_mmioSetBuffer(char *fname)
2.29.2
On 12/17/20 3:27 PM, Andrew Eikum wrote:
Well, you found a new bug. Your tests crash:
https://testbot.winehq.org/JobDetails.pl?Key=83551#k401This is because in mmioOpenW, we allocate only the space required to fit the input filename, but MMIO_PARSE needs room for at least 128 characters. (128 per the docs. Might as well do MAX_PATH.)
I think you need the below diff before applying your patches. It ought to also copy the resulting filename back to the input buffer, but, code freeze.
diff --git a/dlls/winmm/mmio.c b/dlls/winmm/mmio.c index 48bbd3ed48d..4f13a34303c 100644 --- a/dlls/winmm/mmio.c +++ b/dlls/winmm/mmio.c @@ -733,10 +733,9 @@ HMMIO WINAPI mmioOpenW(LPWSTR szFileName, MMIOINFO* lpmmioinfo,
if (szFileName) {
INT len = WideCharToMultiByte( CP_ACP, 0, szFileName, -1, NULL, 0, NULL, NULL );szFn = HeapAlloc( GetProcessHeap(), 0, len );
szFn = HeapAlloc( GetProcessHeap(), 0, MAX_PATH ); if (!szFn) return NULL;
WideCharToMultiByte( CP_ACP, 0, szFileName, -1, szFn, len, NULL, NULL );
WideCharToMultiByte( CP_ACP, 0, szFileName, -1, szFn, MAX_PATH, NULL, NULL );}
ret = MMIO_Open(szFn, lpmmioinfo, dwOpenFlags, TRUE);
I'd be more comfortable with allocating at least MAX_PATH (or 128) bytes rather than always allocating MAX_PATH, especially during code freeze; I've sent a v3 that does this.
I've also written some tests that show the interesting effects of buffer size on MMIO_PARSE...
Andrew
On Thu, Dec 17, 2020 at 02:34:26PM -0600, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura z.figura12@gmail.com
v2: don't assume cwd == executable directory
dlls/winmm/tests/mmio.c | 60 +++++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 8 deletions(-)
diff --git a/dlls/winmm/tests/mmio.c b/dlls/winmm/tests/mmio.c index e3984fbaf3a..0236855f246 100644 --- a/dlls/winmm/tests/mmio.c +++ b/dlls/winmm/tests/mmio.c @@ -478,23 +478,34 @@ static void test_mmioOpen_create(void) WCHAR cwd[MAX_PATH], temp_dir[MAX_PATH]; /* According to docs, filename must be no more than 128 bytes, but it will * actually allow longer than that. */
- WCHAR filename[] = L"very_long_filename_"
WCHAR long_filename[] = L"very_long_filename_" L"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" L"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" L"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx";
WCHAR buffer[MAX_PATH], expect[MAX_PATH];
char exedir_filename[MAX_PATH];
MMIOINFO info = {0};
BOOL ret;
FILE *f;
GetModuleFileNameA(NULL, exedir_filename, ARRAY_SIZE(exedir_filename));
strcpy(strrchr(exedir_filename, '\') + 1, "test_mmio_path");
f = fopen(exedir_filename, "w");
ok(!!f, "failed to create %s: %s\n", debugstr_a(exedir_filename), strerror(errno));
fclose(f);
GetCurrentDirectoryW(ARRAY_SIZE(cwd), cwd); GetTempPathW(ARRAY_SIZE(temp_dir), temp_dir); SetCurrentDirectoryW(temp_dir);
- DeleteFileW(filename);
DeleteFileW(long_filename);
/* open with MMIO_DENYNONE */
- hmmio = mmioOpenW(filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_DENYNONE);
hmmio = mmioOpenW(long_filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_DENYNONE); ok(hmmio != NULL, "mmioOpen failed\n");
/* MMIO_DENYNONE lets us open it here, too */
- handle = CreateFileW(filename, GENERIC_READ,
- handle = CreateFileW(long_filename, GENERIC_READ, FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); ok(handle != INVALID_HANDLE_VALUE, "Couldn't open non-exclusive file\n");
@@ -502,14 +513,14 @@ static void test_mmioOpen_create(void)
mmioClose(hmmio, 0);
- DeleteFileW(filename);
DeleteFileW(long_filename);
/* open with MMIO_EXCLUSIVE */
- hmmio = mmioOpenW(filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_EXCLUSIVE);
hmmio = mmioOpenW(long_filename, NULL, MMIO_CREATE | MMIO_WRITE | MMIO_EXCLUSIVE); ok(hmmio != NULL, "mmioOpen failed\n");
/* should fail due to MMIO_EXCLUSIVE */
- handle = CreateFileW(filename, GENERIC_READ,
- handle = CreateFileW(long_filename, GENERIC_READ, FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); ok(handle == INVALID_HANDLE_VALUE, "Opening exclusive file should have failed\n");
@@ -518,9 +529,42 @@ static void test_mmioOpen_create(void)
mmioClose(hmmio, 0);
- DeleteFileW(filename);
DeleteFileW(long_filename);
wcscpy(buffer, L"test_mmio_path");
hmmio = mmioOpenW(buffer, &info, MMIO_WRITE);
todo_wine ok(!!hmmio, "failed to open file, error %#x\n", info.wErrorRet);
mmioClose(hmmio, 0);
wcscpy(buffer, L"test_mmio_path");
hmmio = mmioOpenW(buffer, &info, MMIO_PARSE);
ok(hmmio == (HMMIO)TRUE, "failed to parse file name, error %#x\n", info.wErrorRet);
wcscpy(expect, temp_dir);
wcscat(expect, L"test_mmio_path");
todo_wine ok(!wcscmp(buffer, expect), "expected %s, got %s\n", debugstr_w(expect), debugstr_w(buffer));
wcscpy(buffer, L"test_mmio_path");
info.wErrorRet = 0xdead;
hmmio = mmioOpenW(buffer, &info, MMIO_EXIST);
ok(hmmio == (HMMIO)FALSE, "file should exist\n");
todo_wine ok(info.wErrorRet == MMIOERR_FILENOTFOUND, "got error %#x\n", info.wErrorRet);
ret = DeleteFileA("test_mmio_path");
ok(!ret, "expected failure\n");
ok(GetLastError() == ERROR_FILE_NOT_FOUND, "got error %u\n", GetLastError());
wcscpy(buffer, L"test_mmio_path");
hmmio = mmioOpenW(buffer, &info, MMIO_WRITE | MMIO_CREATE);
ok(!!hmmio, "failed to open file, error %#x\n", info.wErrorRet);
mmioClose(hmmio, 0);
ret = DeleteFileA("test_mmio_path");
ok(ret, "got error %u\n", GetLastError());
SetCurrentDirectoryW(cwd);
ret = DeleteFileA(exedir_filename);
ok(ret, "got error %u\n", GetLastError());
}
static void test_mmioSetBuffer(char *fname)
2.29.2
-
Andrew Eikum -
Zebediah Figura -
Zebediah Figura (she/her)