March 2017 - Wine-Devel - list.winehq.org

Re: [v3 03/12] comctl32: Added basic implementation for task dialogs and add tests
by Sebastian Lackner March 10, 2017

March 10, 2017

On 10.03.2017 19:21, Fabian Maurer wrote: > + /* Skip this test on wine, because it doesn't really fail, > + * it would displays a dialog that doesn't automatically close */ > + if (strcmp(winetest_platform, "wine")) > + { > + ret = pTaskDialogIndirect(&info, NULL, NULL, NULL); > + ok(ret == S_OK, "Expected S_OK, got %x\n", ret); > + } We usually use winetest_interactive for that. There might be more comments, so please wait a bit before resending. ;) Also, please try to send your changes in reasonable sized chunks (max 5-7) in the future. Sending more patches at once only makes sense if they do not depend on each other. Best regards, Sebastian

3 6

Re: [v3 06/12] comctl32: TaskDialog - make thread-safe
by Nikolay Sivov March 10, 2017

March 10, 2017

On 10.03.2017 21:21, Fabian Maurer wrote: > + > +typedef struct > +{ > + const TASKDIALOGCONFIG *task_config; > + HWND hwnd; > +}taskdialog_info; It doesn't look very safe to keep original pointer here. If you only need callback pointer and user callback data, store them directly.

2 1

Re: [PATCH] dbghelp: Make SymGetLineFromAddr64 call SymGetLineFromAddrW64
by Sebastian Lackner March 10, 2017

March 10, 2017

On 10.03.2017 04:24, Alistair Leslie-Hughes wrote: > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/dbghelp/dbghelp_private.h | 3 ++ > dlls/dbghelp/symbol.c | 96 ++++++++++++++++++++++++++++++------------ > 2 files changed, 73 insertions(+), 26 deletions(-) > Could you give some explanation for the purpose of this patch? It looks a bit odd that we need two almost identical symt_fill_func_line_info implementations. > + if (found) > + { > + char *filename = (char*)source_get(module, dli->u.source_file); > + int len = lstrlenA(filename)+1; > + > + line->FileName = fetch_buffer(module->process, len * sizeof(WCHAR)); > + > + MultiByteToWideChar(CP_ACP, 0, filename, -1, line->FileName, len); [...] > - len = MultiByteToWideChar(CP_ACP, 0, l64->FileName, -1, NULL, 0); > - if ((l64w->FileName = fetch_buffer(pcs, len * sizeof(WCHAR)))) > - MultiByteToWideChar(CP_ACP, 0, l64->FileName, -1, l64w->FileName, len); > - l64w->Address = l64->Address; Actually, the old way of converting the strings looked more reasonable. Computing the correct length and checking for errors is preferred in such cases. Best regards, Sebastian

2 1

Re: [v3 09/12] comctl32: TaskDialog - Changed tests to allow sending messages
by Marvin March 10, 2017

March 10, 2017

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=29422 Your paranoid android. === wvistau64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w2008s64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w7pro64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w864 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w1064 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?)

2 1

Re: [v3 12/12] comctl32: TaskDialog - Add ability to cancel dialog
by Marvin March 10, 2017

March 10, 2017

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=29425 Your paranoid android. === wvistau64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w2008s64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w7pro64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w864 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w1064 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?)

1 0

Re: [v3 11/12] comctl32: TaskDialog - Set default button and add tests
by Marvin March 10, 2017

March 10, 2017

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=29424 Your paranoid android. === wvistau64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w2008s64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w7pro64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w864 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w1064 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?)

1 0

Re: [PATCH 2/2] odbccp32: Implement SQLRemoveDriver/SQLRemoveDriverW
by Vincent Povirk March 10, 2017

March 10, 2017

> + if (usage_count) > + *usage_count = 0; If we're not going to do refcounting properly, it'd make more sense to hard-code this to 1 and never actually remove the drivers. Better to leave things around than delete them while they're still needed.

1 0

Re: [v3 10/12] comctl32: TaskDialog - Add button return value and tests
by Marvin March 10, 2017

March 10, 2017

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=29423 Your paranoid android. === wvistau64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w2008s64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w7pro64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w864 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w1064 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?)

1 0

Re: [PATCH 1/2] odbccp32: Implement SQLInstallDriverEx correctly
by Vincent Povirk March 10, 2017

March 10, 2017

> + if(RegSetValueExW(hkeydriver, driver, 0, REG_SZ, (BYTE*)installed, > + (sizeof(installed)+1)*sizeof(WCHAR)) != ERROR_SUCCESS) sizeof(installed) is already in bytes and includes the null terminator. > + /* Driver and Setup entries use the system path unless a path is specified. */ > + if(lstrcmpiW(driverW, entry) == 0 || lstrcmpiW(setupW, entry) == 0) > + { > + lstrcpyW(value, path); > + lstrcatW(value, slash); > + lstrcatW(value, divider); > + } > + else > + lstrcpyW(value, divider); Some sort of overflow check might be good here. > + ret = SQLInstallDriverEx("WINE ODBC Driver\0Driver=sample.dll\0Setup=sample.dll\0\0", NULL, path, MAX_PATH, &size, ODBC_INSTALL_COMPLETE, NULL); > + sql_ret = SQLInstallerErrorW(1, &error_code, NULL, 0, NULL); > + if (sql_ret && error_code == ODBC_ERROR_WRITING_SYSINFO_FAILED) > + { > + win_skip("not enough privileges\n"); > + return; > + } > + ok(sql_ret && error_code == SQL_SUCCESS, "SQLInstallDriverEx failed %d, %u\n", sql_ret, error_code); > + ok(!strcmp(path, syspath), "invalid path %s\n", path); This doesn't use the value of ret.

1 0

Re: [PATCH 4/5] usp10: Use heap_alloc() instead of HeapAlloc().
by Henri Verbeet March 10, 2017

March 10, 2017

On 10 March 2017 at 16:23, Aric Stewart <aric(a)codeweavers.com> wrote: > Feels a little like there are 2 unrelated changes here the heap_alloc work and then using the loaded_feature temporary variable to clean up the code. I am fine with both changes so I will sign off but i wonder if it makes the commit message a bit less clear. > Yeah, somewhat. It came up during the heap_alloc() work, but ended up touching a fair number of lines. In that regard it could have been split out, although I figured it wasn't too bad.

1 0