[PATCH v2 1/2] winmm/tests: Test the mmioOpen() search path with various flags.
Zebediah Figura (she/her)
zfigura at codeweavers.com
Thu Dec 17 16:41:01 CST 2020
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#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);
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 at 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
>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0x0D9D358A07A17840.asc
Type: application/pgp-keys
Size: 1769 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201217/63fd136c/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201217/63fd136c/attachment.sig>
More information about the wine-devel
mailing list