All patches in this set are for tests that sometimes crash or time out. I'd like to add the checks that Coverity suggests because they might reveal what's going wrong.
-Alex
Alex Henrie (6): kernel32/tests: Add missing return value check to module tests (Coverity) msi/tests: Add missing return value checks to package tests (Coverity) msxml3/tests: Add missing return value checks to domdoc tests (Coverity) shell32/tests: Add missing return value check to shelllink tests (Coverity) shlwapi/tests: Add missing return value checks to istream tests (Coverity) user32/tests: Add missing return value check to msg tests (Coverity)
dlls/kernel32/tests/module.c | 3 ++- dlls/msi/tests/package.c | 13 +++++++++---- dlls/msxml3/tests/domdoc.c | 10 ++++++---- dlls/shell32/tests/shelllink.c | 5 +++-- dlls/shlwapi/tests/istream.c | 6 ++++-- dlls/user32/tests/msg.c | 3 ++- 6 files changed, 26 insertions(+), 14 deletions(-)
Signed-off-by: Alex Henrie alexhenrie24@gmail.com --- dlls/kernel32/tests/module.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/kernel32/tests/module.c b/dlls/kernel32/tests/module.c index 108d0cb999..d3b9c92086 100644 --- a/dlls/kernel32/tests/module.c +++ b/dlls/kernel32/tests/module.c @@ -972,7 +972,8 @@ static void test_AddDllDirectory(void)
buf[0] = '\0'; GetTempPathW( sizeof(path)/sizeof(path[0]), path ); - GetTempFileNameW( path, tmpW, 0, buf ); + ret = GetTempFileNameW( path, tmpW, 0, buf ); + ok( ret, "GetTempFileName failed err %u\n", GetLastError() ); SetLastError( 0xdeadbeef ); cookie = pAddDllDirectory( buf ); ok( cookie != NULL, "AddDllDirectory failed err %u\n", GetLastError() );
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=35151
Your paranoid android.
=== w1064 (64 bit module) === kernel32:module crashed (c00000fd)
2018-01-07 22:12 GMT-07:00 Marvin testbot@winehq.org:
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=35151
Your paranoid android.
=== w1064 (64 bit module) === kernel32:module crashed (c00000fd)
This crash happens intermittently with or without my patch.
-Alex
Signed-off-by: Alex Henrie alexhenrie24@gmail.com --- dlls/msi/tests/package.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/dlls/msi/tests/package.c b/dlls/msi/tests/package.c index 002e6920cf..0f03d2f9ea 100644 --- a/dlls/msi/tests/package.c +++ b/dlls/msi/tests/package.c @@ -3268,7 +3268,8 @@ static void test_states(void) add_file_entry( hdb, "'psi_file', 'psi', 'psi.txt', 100, '', '1033', 8192, 1" ); add_file_entry( hdb, "'upsilon_file', 'upsilon', 'upsilon.txt', 0, '', '1033', 16384, 1" );
- MsiDatabaseCommit(hdb); + r = MsiDatabaseCommit(hdb); + ok( r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", r );
/* these properties must not be in the saved msi file */ add_property_entry( hdb, "'ADDLOCAL', 'one,four'"); @@ -7056,7 +7057,8 @@ static void test_shortlongsource(void) /* CostFinalize:long */ add_directory_entry(hdb, "'SubDir7', 'TARGETDIR', 'eleven|twelve'");
- MsiDatabaseCommit(hdb); + r = MsiDatabaseCommit(hdb); + ok( r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r );
r = package_from_db(hdb, &hpkg); if (r == ERROR_INSTALL_PACKAGE_REJECTED) @@ -8553,7 +8555,8 @@ static void test_MsiEnumComponentCosts(void) add_install_execute_sequence_entry( hdb, "'CostFinalize', '', '1000'" ); add_install_execute_sequence_entry( hdb, "'InstallValidate', '', '1100'" );
- MsiDatabaseCommit( hdb ); + r = MsiDatabaseCommit( hdb ); + ok( r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r );
sprintf( package, "#%u", hdb ); r = MsiOpenPackageA( package, &hpkg ); @@ -9118,6 +9121,7 @@ static INT CALLBACK externalui_message_callback(void *context, UINT message, MSI struct externalui_message msg; char buffer[100]; DWORD length = 100; + UINT r; int i;
msg.message = message; @@ -9132,7 +9136,8 @@ static INT CALLBACK externalui_message_callback(void *context, UINT message, MSI for (i = 0; i <= msg.field_count; i++) { length = 100; - MsiRecordGetStringA(hrecord, i, buffer, &length); + r = MsiRecordGetStringA(hrecord, i, buffer, &length); + ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r); memcpy(msg.field[i], buffer, min(100, length+1)); }
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=35152
Your paranoid android.
=== wxppro (32 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== w2003std (32 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== wvistau64 (32 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== w2008s64 (32 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== w7u (32 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== w7pro64 (32 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== w8 (32 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== w864 (32 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== w1064 (32 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== wvistau64 (64 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== w2008s64 (64 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== w7pro64 (64 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== w864 (64 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
=== w1064 (64 bit package) === package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234 package.c:9140: Test failed: Expected ERROR_SUCCESS, got 234
Signed-off-by: Alex Henrie alexhenrie24@gmail.com --- dlls/msxml3/tests/domdoc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index c1b434bc5f..36fa9535b6 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -6460,7 +6460,8 @@ static void test_save(void) hr = IXMLDOMDocument_loadXML(doc, _bstr_(win1252xml), &b); EXPECT_HR(hr, S_OK);
- CreateStreamOnHGlobal(NULL, TRUE, &stream); + hr = CreateStreamOnHGlobal(NULL, TRUE, &stream); + EXPECT_HR(hr, S_OK); V_VT(&dest) = VT_UNKNOWN; V_UNKNOWN(&dest) = (IUnknown*)stream; hr = IXMLDOMDocument_save(doc, dest); @@ -10950,7 +10951,8 @@ static void test_dispex(void) /* IXMLDOMNodeList */ hr = IXMLDOMDocument_getElementsByTagName(doc, _bstr_("*"), &node_list); EXPECT_HR(hr, S_OK); - IXMLDOMNodeList_QueryInterface(node_list, &IID_IUnknown, (void**)&unk); + hr = IXMLDOMNodeList_QueryInterface(node_list, &IID_IUnknown, (void**)&unk); + EXPECT_HR(hr, S_OK); test_domobj_dispex(unk); IUnknown_Release(unk); IXMLDOMNodeList_Release(node_list); @@ -12094,8 +12096,8 @@ static void test_put_data(void) }
/* compare */ - ok(!lstrcmpW(data, get_data), "%d: got wrong data %s, expected %s\n", *type, wine_dbgstr_w(get_data), - wine_dbgstr_w(data)); + ok(get_data && !lstrcmpW(data, get_data), "%d: got wrong data %s, expected %s\n", + *type, wine_dbgstr_w(get_data), wine_dbgstr_w(data)); SysFreeString(get_data);
IXMLDOMNode_Release(node);
On 01/08/2018 08:05 AM, Alex Henrie wrote:
Signed-off-by: Alex Henrie alexhenrie24@gmail.com
dlls/msxml3/tests/domdoc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index c1b434bc5f..36fa9535b6 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -6460,7 +6460,8 @@ static void test_save(void) hr = IXMLDOMDocument_loadXML(doc, _bstr_(win1252xml), &b); EXPECT_HR(hr, S_OK);
- CreateStreamOnHGlobal(NULL, TRUE, &stream);
- hr = CreateStreamOnHGlobal(NULL, TRUE, &stream);
- EXPECT_HR(hr, S_OK); V_VT(&dest) = VT_UNKNOWN; V_UNKNOWN(&dest) = (IUnknown*)stream; hr = IXMLDOMDocument_save(doc, dest);
@@ -10950,7 +10951,8 @@ static void test_dispex(void) /* IXMLDOMNodeList */ hr = IXMLDOMDocument_getElementsByTagName(doc, _bstr_("*"), &node_list); EXPECT_HR(hr, S_OK);
- IXMLDOMNodeList_QueryInterface(node_list, &IID_IUnknown, (void**)&unk);
- hr = IXMLDOMNodeList_QueryInterface(node_list, &IID_IUnknown, (void**)&unk);
- EXPECT_HR(hr, S_OK); test_domobj_dispex(unk); IUnknown_Release(unk); IXMLDOMNodeList_Release(node_list);
@@ -12094,8 +12096,8 @@ static void test_put_data(void) }
Please unwrap it to a regular ok().
/* compare */
ok(!lstrcmpW(data, get_data), "%d: got wrong data %s, expected %s\n", *type, wine_dbgstr_w(get_data),wine_dbgstr_w(data));
ok(get_data && !lstrcmpW(data, get_data), "%d: got wrong data %s, expected %s\n",*type, wine_dbgstr_w(get_data), wine_dbgstr_w(data)); SysFreeString(get_data); IXMLDOMNode_Release(node);
Do we have actual crashes because of that?
2018-01-07 23:38 GMT-07:00 Nikolay Sivov nsivov@codeweavers.com:
/* compare */
ok(!lstrcmpW(data, get_data), "%d: got wrong data %s, expected%s\n", *type, wine_dbgstr_w(get_data),
wine_dbgstr_w(data));
ok(get_data && !lstrcmpW(data, get_data), "%d: got wrong data %s,expected %s\n",
*type, wine_dbgstr_w(get_data), wine_dbgstr_w(data)); SysFreeString(get_data); IXMLDOMNode_Release(node);Do we have actual crashes because of that?
Interesting, Coverity thinks that lstrcmpW doesn't check for null pointers, but Wine's implementation of it does. I'll have to do more testing to see who's correct.
-Alex
Signed-off-by: Alex Henrie alexhenrie24@gmail.com --- dlls/shell32/tests/shelllink.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/dlls/shell32/tests/shelllink.c b/dlls/shell32/tests/shelllink.c index 6f12fb2687..4f1629090f 100644 --- a/dlls/shell32/tests/shelllink.c +++ b/dlls/shell32/tests/shelllink.c @@ -169,8 +169,9 @@ static void test_get_set(void) ok(finddata.dwFileAttributes == 0, "unexpected attributes %x\n", finddata.dwFileAttributes); ok(finddata.cFileName[0] == 0, "unexpected filename '%s'\n", finddata.cFileName);
- CoCreateInstance(&CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER, - &IID_IShellLinkW, (LPVOID*)&slW); + r = CoCreateInstance(&CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER, + &IID_IShellLinkW, (LPVOID*)&slW); + ok(r == S_OK, "CoCreateInstance failed (0x%08x)\n", r); if (!slW /* Win9x */ || !pGetLongPathNameA /* NT4 */) skip("SetPath with NULL parameter crashes on Win9x and some NT4\n"); else
Signed-off-by: Alex Henrie alexhenrie24@gmail.com --- dlls/shlwapi/tests/istream.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/dlls/shlwapi/tests/istream.c b/dlls/shlwapi/tests/istream.c index 54a73fcf5e..f49ddd1824 100644 --- a/dlls/shlwapi/tests/istream.c +++ b/dlls/shlwapi/tests/istream.c @@ -663,8 +663,10 @@ static void test_SHCreateStreamOnFileEx_CopyTo(void) static const WCHAR prefix[] = { 'T', 'S', 'T', 0 };
GetTempPathW(MAX_PATH, tmpPath); - GetTempFileNameW(tmpPath, prefix, 0, srcFileName); - GetTempFileNameW(tmpPath, prefix, 0, dstFileName); + ret = GetTempFileNameW(tmpPath, prefix, 0, srcFileName); + ok(SUCCEEDED(ret), "GetTempFileName failed, got error %d\n", GetLastError()); + ret = GetTempFileNameW(tmpPath, prefix, 0, dstFileName); + ok(SUCCEEDED(ret), "GetTempFileName failed, got error %d\n", GetLastError());
ret = SHCreateStreamOnFileEx(srcFileName, STGM_CREATE | STGM_READWRITE | STGM_DELETEONRELEASE, FILE_ATTRIBUTE_TEMPORARY, FALSE, NULL, &src); ok(SUCCEEDED(ret), "SHCreateStreamOnFileEx failed with ret=0x%08x\n", ret);
On 01/08/2018 08:05 AM, Alex Henrie wrote:
Signed-off-by: Alex Henrie alexhenrie24@gmail.com
dlls/shlwapi/tests/istream.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/dlls/shlwapi/tests/istream.c b/dlls/shlwapi/tests/istream.c index 54a73fcf5e..f49ddd1824 100644 --- a/dlls/shlwapi/tests/istream.c +++ b/dlls/shlwapi/tests/istream.c @@ -663,8 +663,10 @@ static void test_SHCreateStreamOnFileEx_CopyTo(void) static const WCHAR prefix[] = { 'T', 'S', 'T', 0 };
GetTempPathW(MAX_PATH, tmpPath);
- GetTempFileNameW(tmpPath, prefix, 0, srcFileName);
- GetTempFileNameW(tmpPath, prefix, 0, dstFileName);
- ret = GetTempFileNameW(tmpPath, prefix, 0, srcFileName);
- ok(SUCCEEDED(ret), "GetTempFileName failed, got error %d\n", GetLastError());
- ret = GetTempFileNameW(tmpPath, prefix, 0, dstFileName);
- ok(SUCCEEDED(ret), "GetTempFileName failed, got error %d\n", GetLastError());
Return value is not HRESULT code in this case.
ret = SHCreateStreamOnFileEx(srcFileName, STGM_CREATE | STGM_READWRITE | STGM_DELETEONRELEASE, FILE_ATTRIBUTE_TEMPORARY, FALSE, NULL, &src); ok(SUCCEEDED(ret), "SHCreateStreamOnFileEx failed with ret=0x%08x\n", ret);
Signed-off-by: Alex Henrie alexhenrie24@gmail.com --- dlls/user32/tests/msg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 1ccf0d4a36..52384b526c 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -13104,7 +13104,8 @@ static void test_dialog_messages(void) SetFocus(child);
flush_sequence(); - DialogBoxA( 0, "TEST_DIALOG", child2, TestModalDlgProc2 ); + ret = DialogBoxA( 0, "TEST_DIALOG", child2, TestModalDlgProc2 ); + ok (ret != 0, "DialogBox error %u\n", GetLastError()); ok_sequence(WmModalDialogSeq_2, "ModalDialog2", TRUE);
DestroyWindow(child2);
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=35156
Your paranoid android.
=== wxppro (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w2003std (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== wvistau64 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w2008s64 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w7u (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w7pro64 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w8 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w864 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w1064 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== wvistau64 (64 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w2008s64 (64 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w7pro64 (64 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w864 (64 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w1064 (64 bit msg) === msg.c:13109: Test failed: DialogBox error 0
2018-01-07 22:35 GMT-07:00 Marvin testbot@winehq.org:
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=35156
Your paranoid android.
=== wxppro (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w2003std (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== wvistau64 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w2008s64 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w7u (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w7pro64 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w8 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w864 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w1064 (32 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== wvistau64 (64 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w2008s64 (64 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w7pro64 (64 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w864 (64 bit msg) === msg.c:13109: Test failed: DialogBox error 0
=== w1064 (64 bit msg) === msg.c:13109: Test failed: DialogBox error 0
I forgot to run these patches through the testbot before submitting, sorry. This patch should not have been included in the series at all.
-Alex
-
Alex Henrie -
Marvin -
Nikolay Sivov