September 2018 - Wine-Devel

Re: [PATCH] scrrun: Add MoveFile().

by Nikolay Sivov

On 09/16/2018 04:07 PM, Marvin wrote: > 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=42077 > > Your paranoid android. > > > === wvistau64_zh_CN (32 bit Windows report) === > > scrrun: > filesystem.c:1530: Test failed: got L"\f8f5", expected L"\f8f5?" > filesystem.c:1659: Test failed: got L"\f8f5\fa0f", 125 > filesystem.c:1806: Test failed: got L"\f8f5\fa0f", expected L"\f8f5?" This is an existing bug https://bugs.winehq.org/show_bug.cgi?id=39244.

Sept. 16, 2018

Re: [PATCH] find: First simple implementation and tests

by Nikolay Sivov

On 09/15/2018 11:38 PM, Fabian Maurer wrote: > > > + /* Read next content into buffer */ > > > > + if (buffer_pos >= buffer_max) > > > > + { > > > > + BOOL success = ReadFile(handle, buffer, 4096, &buffer_max, NULL); > > > > + if (!success) > > > > > > Separate variable does not seem to be necessary. > > BOOL success you mean? I always pull that out of an if to make it more > readable. > > > > + line_converted_length = MultiByteToWideChar(CP_ACP, 0, line, -1, 0, > > > > 0); + line_converted = heap_alloc(line_converted_length * > > > > sizeof(WCHAR)); + MultiByteToWideChar(CP_ACP, 0, line, -1, > > > > line_converted, line_converted_length); + > > > > + heap_free(line); > > > > + > > > > + *line_out = line_converted; > > > > + return TRUE; > > > > +} > > > > > > CP_ACP is probably not correct in this case. > > Maybe, but it works in the tests for now. What would you propose? > Again, take a look how output works in reg or regedit. > > > > > > +void write_to_handle(HANDLE handle, const WCHAR *str) > > > > +{ > > > > + DWORD bytes_written_sum = 0; > > > > + char *str_converted; > > > > + UINT str_converted_length; > > > > + UINT str_length = lstrlenW(str); > > > > + > > > > + str_converted_length = WideCharToMultiByte(CP_ACP, 0, str, > > > > str_length, NULL, 0, NULL, NULL); + str_converted = > > > > heap_alloc(str_converted_length); > > > > + WideCharToMultiByte(CP_ACP, 0, str, str_length, str_converted, > > > > str_converted_length, NULL, NULL); + > > > > + do > > > > + { > > > > + DWORD bytes_written; > > > > + WriteFile(handle, str_converted, str_converted_length, > > > > &bytes_written, NULL); + bytes_written_sum += bytes_written; > > > > + } while (bytes_written_sum < str_converted_length); > > > > + > > > > + heap_free(str_converted); > > > > +} > > > > + > > > > +void find_printf(const WCHAR *str) > > > > +{ > > > > + write_to_handle(GetStdHandle(STD_OUTPUT_HANDLE), str); > > > > +} > > > > > > Check how it's done in reg/regedit, probably you should reuse it here. > > > Less important, but why two helpers here? And why do you need a loop to > > > write this buffer? > > I'll take a look at it. AFAIK WriteFile doesn't guarantee all bytes to > be written at once, that's why we have the "bytes written" parameter. > Correct me if I'm wrong. > I think it's guaranteed for synchronous case. > > There's strstrW(), importing shlwapi only for that is too much. > > Ah, missed that one. I'll change it. > > > > > > > + WCHAR message_parameter_invalid[64]; > > > > + WCHAR message_switch_invalid[64]; > > > > > > > > int i; > > > > > > > > + LoadStringW(GetModuleHandleW(NULL), IDS_INVALID_PARAMETER, > > > > message_parameter_invalid, sizeof(message_parameter_invalid)); + > > > > LoadStringW(GetModuleHandleW(NULL), IDS_INVALID_SWITCH, > > > > message_switch_invalid, sizeof(message_switch_invalid)); > > > You don't need them both at the same time. Also check the arguments. > > Sure thing, just loaded all of them at once for simplicity. > Performance wise it shouldn't matter, but do you want me to load them > only when needed? > > By checking the arguments, do you mean I should call it with buffer > length 0 to get a readonly pointer? > I mean length is wrong. > > > > > > + input = GetStdHandle(STD_INPUT_HANDLE); > > > > + > > > > + for (i = 1; i < argc; i++) > > > > + { > > > > + if (argv[i][0] == '/') > > > > + { > > > > + find_printf(message_switch_invalid); > > > > + return 2; > > > > + } > > > > + else if(tofind == NULL) > > > > + { > > > > + tofind = argv[i]; > > > > + } > > > > + } > > > > + > > > > + if (tofind == NULL) > > > > + { > > > > + find_printf(message_parameter_invalid); > > > > + return 2; > > > > + } > > > > + > > > > + exitcode = 1; > > > > + while (read_line_from_handle(input, &line)) > > > > + { > > > > + if (run_find_for_line(line, tofind)) > > > > + exitcode = 0; > > > > + > > > > + heap_free(line); > > > > + } > > > > > > I don't understand why you're reading from stdin uncoditionally. This > > > utility takes options, string to find, and a list of file paths. Only > > > when paths are not provided it should read from stdin. Will this work if > > > you have your string argument starts with '/'? > > It's only supposed to be a first implementation, I don't want to add > everything in one huge commit. It's already gotten too big for my > likes, I don't think we should add even more functionality for the > first version. > I'm not saying it should support everything from the start, but handling of command line arguments is important. It should print some warnings and/or abort if file paths are supplied, otherwise it will silently return nothing. > Regards, > > Fabian Maurer >

Sept. 16, 2018

Re: [PATCH] scrrun: Add MoveFile().

by Marvin

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=42077 Your paranoid android. === wvistau64_zh_CN (32 bit Windows report) === scrrun: filesystem.c:1530: Test failed: got L"\f8f5", expected L"\f8f5?" filesystem.c:1659: Test failed: got L"\f8f5\fa0f", 125 filesystem.c:1806: Test failed: got L"\f8f5\fa0f", expected L"\f8f5?"

Sept. 16, 2018

Re: [PATCH] comctl32/taskdialog: Use parentheses around & operator

by Nikolay Sivov

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>

Sept. 16, 2018

[PATCH] scrrun: Add MoveFile().

by Nikolay Sivov

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/scrrun/filesystem.c | 21 +++++++++++++++++---- dlls/scrrun/tests/filesystem.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c index aa1660085e..daac75f1e1 100644 --- a/dlls/scrrun/filesystem.c +++ b/dlls/scrrun/filesystem.c @@ -3596,12 +3596,25 @@ static HRESULT WINAPI filesys_DeleteFolder(IFileSystem3 *iface, BSTR FolderSpec, return delete_folder(FolderSpec, SysStringLen(FolderSpec), Force); } -static HRESULT WINAPI filesys_MoveFile(IFileSystem3 *iface, BSTR Source, - BSTR Destination) +static HRESULT WINAPI filesys_MoveFile(IFileSystem3 *iface, BSTR src, BSTR dest) { - FIXME("%p %s %s\n", iface, debugstr_w(Source), debugstr_w(Destination)); + DWORD error; - return E_NOTIMPL; + TRACE("%p %s %s\n", iface, debugstr_w(src), debugstr_w(dest)); + + if (MoveFileW(src, dest)) + return S_OK; + + error = GetLastError(); + switch (error) + { + case ERROR_ALREADY_EXISTS: + return CTL_E_FILEALREADYEXISTS; + case ERROR_FILE_NOT_FOUND: + return CTL_E_FILENOTFOUND; + default: + return HRESULT_FROM_WIN32(error); + } } static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface,BSTR Source, diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c index 29ee73f243..925a889a3a 100644 --- a/dlls/scrrun/tests/filesystem.c +++ b/dlls/scrrun/tests/filesystem.c @@ -2217,6 +2217,39 @@ static void test_GetSpecialFolder(void) IFolder_Release(folder); } +static void test_MoveFile(void) +{ + WCHAR source[MAX_PATH], dest[MAX_PATH], tempdir[MAX_PATH]; + static const WCHAR prefixW[] = {'p','f','x',0}; + BSTR str1, str2; + HRESULT hr; + DWORD ret; + + ret = GetTempPathW(ARRAY_SIZE(tempdir), tempdir); + ok(ret != 0, "Failed to get temp path, error %d\n", GetLastError()); + + ret = GetTempFileNameW(tempdir, prefixW, 0, source); + ok(ret != 0, "Failed to get temp name, error %d\n", GetLastError()); + + ret = GetTempFileNameW(tempdir, prefixW, 0, dest); + ok(ret != 0, "Failed to get temp name, error %d\n", GetLastError()); + + str1 = SysAllocString(source); + str2 = SysAllocString(dest); + + hr = IFileSystem3_MoveFile(fs3, str1, str2); + ok(hr == CTL_E_FILEALREADYEXISTS, "Unexpected hr %#x.\n", hr); + + DeleteFileW(source); + DeleteFileW(dest); + + hr = IFileSystem3_MoveFile(fs3, str1, str2); + ok(hr == CTL_E_FILENOTFOUND, "Unexpected hr %#x.\n", hr); + + SysFreeString(str1); + SysFreeString(str2); +} + START_TEST(filesystem) { HRESULT hr; @@ -2255,6 +2288,7 @@ START_TEST(filesystem) test_SerialNumber(); test_GetExtensionName(); test_GetSpecialFolder(); + test_MoveFile(); IFileSystem3_Release(fs3); -- 2.18.0

Sept. 16, 2018

[PATCH] oleview: Add missing break (coverity)

by Fabian Maurer

Not 100% sure, but I guess this wasn't intended. If it was, it would warrant a comment, at least. Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- programs/oleview/typelib.c | 1 + 1 file changed, 1 insertion(+) diff --git a/programs/oleview/typelib.c b/programs/oleview/typelib.c index 4912a5fb00..99de87b43b 100644 --- a/programs/oleview/typelib.c +++ b/programs/oleview/typelib.c @@ -1598,6 +1598,7 @@ static LRESULT CALLBACK TypeLibProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM } case WM_COMMAND: TypeLibMenuCommand(LOWORD(wParam), hWnd); + break; case WM_MENUSELECT: UpdateTypeLibStatusBar(LOWORD(wParam)); break; -- 2.19.0

Sept. 16, 2018

[PATCH] opcservices: Add missing break (coverity)

by Fabian Maurer

Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/opcservices/package.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/opcservices/package.c b/dlls/opcservices/package.c index eb9c1c9f5a..f3503945b5 100644 --- a/dlls/opcservices/package.c +++ b/dlls/opcservices/package.c @@ -607,6 +607,7 @@ static HRESULT WINAPI opc_content_stream_Seek(IStream *iface, LARGE_INTEGER move break; case STREAM_SEEK_END: pos.QuadPart = stream->content->size.QuadPart + move.QuadPart; + break; default: WARN("Unknown origin mode %d.\n", origin); return E_INVALIDARG; -- 2.19.0

Sept. 16, 2018

[PATCH] windowscodecs: IWICMetadataQueryReader::GetMetadataByName() should accept NULL for returned value.

by Dmitry Timoshkov

Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/windowscodecs/metadataquery.c | 2 +- dlls/windowscodecs/tests/metadata.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/windowscodecs/metadataquery.c b/dlls/windowscodecs/metadataquery.c index 2d7c918921..c4c188d8c9 100644 --- a/dlls/windowscodecs/metadataquery.c +++ b/dlls/windowscodecs/metadataquery.c @@ -589,7 +589,7 @@ static HRESULT WINAPI mqr_GetMetadataByName(IWICMetadataQueryReader *iface, LPCW PropVariantClear(&tk_id); PropVariantClear(&tk_schema); - if (hr == S_OK) + if (hr == S_OK && value) *value = new_value; else PropVariantClear(&new_value); diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c index eb846db0b8..b434648fe5 100644 --- a/dlls/windowscodecs/tests/metadata.c +++ b/dlls/windowscodecs/tests/metadata.c @@ -1606,6 +1606,10 @@ static void test_metadata_gif(void) if (winetest_debug > 1) trace("query: %s\n", decoder_data[i].query); MultiByteToWideChar(CP_ACP, 0, decoder_data[i].query, -1, queryW, 256); + + hr = IWICMetadataQueryReader_GetMetadataByName(queryreader, queryW, NULL); + ok(hr == decoder_data[i].hr, "GetMetadataByName(%s) returned %#x, expected %#x\n", wine_dbgstr_w(queryW), hr, decoder_data[i].hr); + PropVariantInit(&value); hr = IWICMetadataQueryReader_GetMetadataByName(queryreader, queryW, &value); ok(hr == decoder_data[i].hr, "GetMetadataByName(%s) returned %#x, expected %#x\n", wine_dbgstr_w(queryW), hr, decoder_data[i].hr); -- 2.17.1

Sept. 16, 2018

Wine staging 3.16 release

by Alistair Leslie-Hughes

Binary packages for various distributions will be available from: https://www.winehq.org/download Summary of the past few weeks * Rebased to current wine 3.16 (880 patches are applied to wine vanilla) * Reviewed and upstreamed patches. Added * [34711] hnetcfg: Add Support for interface UPnPNAT Where can you help * Run Steam/Battle.net/UPlay. * Try you favorite game: Terraria * Test your favorite applications. * Steal staging patches and get them accepted upstream. As always, if you find a bug, please report it via https://bugs.winehq.org Best Regards Alistair.

Sept. 16, 2018

Re: [PATCH] find: First simple implementation and tests

by Fabian Maurer

On Samstag, 15. September 2018 20:42:46 CEST Nikolay Sivov wrote: > On 09/15/2018 06:42 PM, Fabian Maurer wrote: > > The message text is the same as on windows, > > since the exit code doesn't seem reliable. > > Is it unreliable across Windows versions? I only see error message for > invalid input, not for the case when string was or wasn't found. So why > does it matter? I might have mixed that up with fc.exe, I'll take a second look. > > +BOOL read_char_from_handle(HANDLE handle, char *char_out) > > Function name does not correspond with what function is doing. You mean because it does buffering, too? > > + static char buffer[4096]; > > + static UINT buffer_max = 0; > > + static UINT buffer_pos = 0; > > What's a reason for this to be static? So it's not lost when the function returns. We read a lot from the handle, but only use one char at a time. This buffering is needed since reading one char at a time with ReadFile would be to inefficient. > > + /* Read next content into buffer */ > > + if (buffer_pos >= buffer_max) > > + { > > + BOOL success = ReadFile(handle, buffer, 4096, &buffer_max, NULL); > > + if (!success) > > Separate variable does not seem to be necessary. BOOL success you mean? I always pull that out of an if to make it more readable. > > + line_converted_length = MultiByteToWideChar(CP_ACP, 0, line, -1, 0, > > 0); + line_converted = heap_alloc(line_converted_length * > > sizeof(WCHAR)); + MultiByteToWideChar(CP_ACP, 0, line, -1, > > line_converted, line_converted_length); + > > + heap_free(line); > > + > > + *line_out = line_converted; > > + return TRUE; > > +} > > CP_ACP is probably not correct in this case. Maybe, but it works in the tests for now. What would you propose? > > > +void write_to_handle(HANDLE handle, const WCHAR *str) > > +{ > > + DWORD bytes_written_sum = 0; > > + char *str_converted; > > + UINT str_converted_length; > > + UINT str_length = lstrlenW(str); > > + > > + str_converted_length = WideCharToMultiByte(CP_ACP, 0, str, > > str_length, NULL, 0, NULL, NULL); + str_converted = > > heap_alloc(str_converted_length); > > + WideCharToMultiByte(CP_ACP, 0, str, str_length, str_converted, > > str_converted_length, NULL, NULL); + > > + do > > + { > > + DWORD bytes_written; > > + WriteFile(handle, str_converted, str_converted_length, > > &bytes_written, NULL); + bytes_written_sum += bytes_written; > > + } while (bytes_written_sum < str_converted_length); > > + > > + heap_free(str_converted); > > +} > > + > > +void find_printf(const WCHAR *str) > > +{ > > + write_to_handle(GetStdHandle(STD_OUTPUT_HANDLE), str); > > +} > > Check how it's done in reg/regedit, probably you should reuse it here. > Less important, but why two helpers here? And why do you need a loop to > write this buffer? I'll take a look at it. AFAIK WriteFile doesn't guarantee all bytes to be written at once, that's why we have the "bytes written" parameter. Correct me if I'm wrong. > There's strstrW(), importing shlwapi only for that is too much. Ah, missed that one. I'll change it. > > > + WCHAR message_parameter_invalid[64]; > > + WCHAR message_switch_invalid[64]; > > > > int i; > > > > + LoadStringW(GetModuleHandleW(NULL), IDS_INVALID_PARAMETER, > > message_parameter_invalid, sizeof(message_parameter_invalid)); + > > LoadStringW(GetModuleHandleW(NULL), IDS_INVALID_SWITCH, > > message_switch_invalid, sizeof(message_switch_invalid)); > You don't need them both at the same time. Also check the arguments. Sure thing, just loaded all of them at once for simplicity. Performance wise it shouldn't matter, but do you want me to load them only when needed? By checking the arguments, do you mean I should call it with buffer length 0 to get a readonly pointer?

Sept. 15, 2018