July 2017 - Wine-devel - list.winehq.org

Re: [PATCH 3/3] mshtml: Expose IHTMLDOMNode3 interface to scripts.
by Marvin 19 Jul '17

19 Jul '17

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=32387 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath

1 0

Re: [v2] gdi32: return non-zero v-refresh value for display devices.
by Huw Davies 19 Jul '17

19 Jul '17

On Wed, Jul 19, 2017 at 11:28:58PM +0900, Akihiro Sagawa wrote: > diff --git a/dlls/gdi32/tests/dc.c b/dlls/gdi32/tests/dc.c > index add2c04..2b48542 100644 > --- a/dlls/gdi32/tests/dc.c > +++ b/dlls/gdi32/tests/dc.c > @@ -445,6 +445,7 @@ static void test_device_caps( HDC hdc, HDC ref_dc, const char *descr, int scale > if (type != COMPLEXREGION && type != ERROR) /* region can be complex on multi-monitor setups */ > { > RECT ref_rect; > + int rate = GetDeviceCaps( ref_dc, VREFRESH ); > > ok( type == SIMPLEREGION, "GetClipBox returned %d on %s\n", type, descr ); > if (GetDeviceCaps( ref_dc, TECHNOLOGY ) == DT_RASDISPLAY) > @@ -462,11 +463,15 @@ static void test_device_caps( HDC hdc, HDC ref_dc, const char *descr, int scale > SetRect( &ref_rect, GetSystemMetrics( SM_XVIRTUALSCREEN ), GetSystemMetrics( SM_YVIRTUALSCREEN ), > GetSystemMetrics( SM_XVIRTUALSCREEN ) + GetSystemMetrics( SM_CXVIRTUALSCREEN ), > GetSystemMetrics( SM_YVIRTUALSCREEN ) + GetSystemMetrics( SM_CYVIRTUALSCREEN ) ); > + > + ok( rate >= 1 && rate <= 85, "returned rate is out of range (1 -- 85), got %d\n", rate); Don't check the range, just that it's != 0. Also, please move this to a new VREFRESH case in the switch block higher up in this function. Huw.

1 0

Re: winhttp: Rename host_t to hostdata_t.
by Henri Verbeet 19 Jul '17

19 Jul '17

On 19 July 2017 at 17:22, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > Commit fa2cc9868b7db7a4cbf7d01a48500cb3f31e109a broke the compilation macOS > because host_t conflicts with a system header file: > > [19:31:41] In file included from ../../../wine/dlls/winhttp/session.c:50: > [19:31:41] ../../../wine/dlls/winhttp/winhttp_private.h:106:3: error: typedef redefinition with different types ('struct host_t' vs 'mach_port_t' (aka 'unsigned int')) > [19:31:41] } host_t; > [19:31:41] ^ > [19:31:41] /build/macos-rootfs/usr/include/mach/mach_types.h:120:22: note: previous definition is here > [19:31:41] typedef mach_port_t host_t; > [19:31:41] ^ > It may be too late for winhttp or Wine in general (although Michael does love these kinds of things), but note that the _t suffix is reserved by POSIX and should in principle be avoided on POSIX systems.

1 0

Re: gdi32: return non-zero v-refresh value for display devices.
by Huw Davies 19 Jul '17

19 Jul '17

On Wed, Jul 19, 2017 at 12:11:28AM +0900, Akihiro Sagawa wrote: > > To avoid division by zero (Bug 43369). > > Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com> > --- > dlls/gdi32/driver.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/gdi32/driver.c b/dlls/gdi32/driver.c > index fe7812c..6a3975a 100644 > --- a/dlls/gdi32/driver.c > +++ b/dlls/gdi32/driver.c > @@ -344,7 +344,7 @@ static INT nulldrv_GetDeviceCaps( PHYSDEV dev, INT cap ) > case PHYSICALOFFSETY: return 0; > case SCALINGFACTORX: return 0; > case SCALINGFACTORY: return 0; > - case VREFRESH: return 0; > + case VREFRESH: return GetDeviceCaps( dev->hdc, TECHNOLOGY ) == DT_RASDISPLAY ? 1 : 0; > case DESKTOPVERTRES: return GetDeviceCaps( dev->hdc, VERTRES ); > case DESKTOPHORZRES: return GetDeviceCaps( dev->hdc, HORZRES ); > case BLTALIGNMENT: return 0; Could you add a test (probably in dc.c:test_device_caps())? The values returned by the testbot range from 1 -- 85, so 1 is as good a choice as any. Huw.

2 2

Re: [v2 3/5] d3dx9: Support skip_constants parameter for effect.
by Matteo Bruni 18 Jul '17

18 Jul '17

2017-07-11 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > @@ -6397,24 +6464,75 @@ static HRESULT d3dx9_base_effect_init(struct d3dx9_base_effect *base, > TRACE("Tag: %x\n", tag); > } > > + if (skip_constants_string) > + { > + skip_constants_buffer = HeapAlloc(GetProcessHeap(), 0, > + sizeof(*skip_constants_buffer) * (strlen(skip_constants_string) + 1)); I think you can get rid of the sizeof() in this case. If sizeof(*skip_constants_buffer) is not 1 we have bigger problems anyway... > + if (!skip_constants_buffer) > + { > + if (bytecode) > + ID3D10Blob_Release(bytecode); > + return E_OUTOFMEMORY; > + } > + strcpy(skip_constants_buffer, skip_constants_string); Ideally this one should be a memcpy(), reusing the string length computed above. > @@ -588,7 +588,8 @@ static unsigned int *parse_pres_ins(unsigned int *ptr, unsigned int count, struc > return ptr; > } > > -static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D3DXCONSTANT_DESC *desc) > +static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D3DXCONSTANT_DESC *desc, > + WORD *constantinfo_reserved) > { > const struct ctab_constant *constant = d3dx_shader_get_ctab_constant(ctab, hc); > > @@ -598,10 +599,13 @@ static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D > return D3DERR_INVALIDCALL; > } > *desc = constant->desc; > + if (constantinfo_reserved) > + *constantinfo_reserved = constant->constantinfo_reserved; > return D3D_OK; > } For future reference: these (and related) changes should probably have been in a separate patch, or even part of the previous patch in the series. Also, as it becomes more obvious here, one could go one step further with this and directly return struct ctab_constant to the caller. That would essentially make the get_ctab_constant_desc() helper redundant so it could be merged into the callers and removed entirely. > + for (j = 0; j < skip_constants_count; ++j) > + { > + if (!strcmp(cdesc[index].Name, skip_constants[j])) > + { > + if (!constantinfo_reserved) > + { > + WARN("skip_constants parameter %s is not register bound.\n", > + cdesc[index].Name); > + hr = D3DERR_INVALIDCALL; > + goto cleanup; > + } > + TRACE("Skipping constant %s.\n", cdesc[index].Name); > + break; > + } > + } > + if (j < skip_constants_count) > + continue; > + ++out->input_count; > + if (inputs_param[index]->class == D3DXPC_OBJECT) > + continue; > + if (FAILED(hr = init_set_constants_param(out, ctab, hc, inputs_param[index]))) > + goto cleanup; > } > - out->input_count = desc.Constants; > if (out->const_set_count) > { > out->const_set = HeapReAlloc(GetProcessHeap(), 0, out->const_set, Not particularly important but we could shrink out->inputs and out->inputs_param to out->input_count if any parameter was skipped.

1 0

Re: [PATCH 7/7] d3dx9: Support skip_constants parameter for effect.
by Matteo Bruni 18 Jul '17

18 Jul '17

2017-07-06 13:15 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > HRESULT d3dx_create_param_eval(struct d3dx9_base_effect *base_effect, void *byte_code, > unsigned int byte_code_size, D3DXPARAMETER_TYPE type, > - struct d3dx_param_eval **peval, ULONG64 *version_counter) DECLSPEC_HIDDEN; > + struct d3dx_param_eval **peval, ULONG64 *version_counter, > + const char **skip_constants_names, unsigned int skip_constant_name_count) DECLSPEC_HIDDEN; I think you can get rid of the "_name" part in those parameters, for brevity. > +static HRESULT parse_skip_constants_string(char *skip_constants, const char ***names, > + unsigned int *name_count) It's a bit a matter of personal taste but triple pointers look quite ugly to me. In this case you could e.g. return "names" in place of HRESULT. > +{ > + const char *name; > + char *s; > + unsigned int size = INITIAL_CONST_NAMES_SIZE; > + > + *names = HeapAlloc(GetProcessHeap(), 0, sizeof(*names) * size); > + if (!*names) > + return E_OUTOFMEMORY; > + > + *name_count = 0; > + s = skip_constants; > + while ((name = next_valid_constant_name(&s))) > + { > + if (*name_count == size) > + { > + size *= 2; > + *names = HeapReAlloc(GetProcessHeap(), 0, *names, sizeof(*names) * size); > + if (!*names) > + return E_OUTOFMEMORY; I think you're leaking memory on failure here. > + } > + (*names)[(*name_count)++] = name; > + } > + *names = HeapReAlloc(GetProcessHeap(), 0, *names, *name_count * sizeof(*names)); > + if (!*names) > + return E_OUTOFMEMORY; And here. > + for (i = 0; i < skip_constants_name_count; ++i) > + { > + struct d3dx_parameter *param; > + > + param = get_parameter_by_name(base, NULL, skip_constants_names[i]); > + if (param) > + { > + for (j = 0; j < base->technique_count; ++j) > + { > + if (is_parameter_used(param, &base->techniques[j])) > + { > + WARN("Parameter %s found in skip_constants is used in technique %u.\n", > + debugstr_a(skip_constants_names[i]), j); > + HeapFree(GetProcessHeap(), 0, skip_constants_buffer); > + HeapFree(GetProcessHeap(), 0, skip_constants_names); > + d3dx9_base_effect_cleanup(base); > + return D3DERR_INVALIDCALL; > + } > + } > + } > + else > + { > + TRACE("Parameter %s found in skip_constants not found.\n", > + debugstr_a(skip_constants_names[i])); This one sounds a bit awkward ("found... not found"). Maybe just rewrite that as "skip_constants parameter %s not found". You could apply the same trick to the previous message too. > + for (j = 0; j < skip_constant_name_count; ++j) > + { > + if (!strcmp(cdesc[index].Name, skip_constants_names[j])) I assume the skip_constants thing isn't used often and, when it is, very few constants (1-3) are in the list. If that's not the case, we might want to use an rbtree for the constant names.

2 2

Re: [PATCH v2] user32: should restore owned window when restoring owner window.
by Andrew Eikum 18 Jul '17

18 Jul '17

On Tue, Jul 18, 2017 at 04:47:05PM +0800, jiang.biao2(a)zte.com.cn wrote: > Is this the right place to commit patches for wine? > > could somebody help to review this patch? > This is the right place to send patches. Your patch needs tests to show that it is correct, and to detect regressions from future changes. Here are some good places to start: https://wiki.winehq.org/Developer_FAQ https://wiki.winehq.org/Submitting_Patches https://wiki.winehq.org/Conformance_Tests Andrew

1 0

Re: [PATCH 4/5] msvcr120: Don't set errno in expm1.
by Piotr Caban 18 Jul '17

18 Jul '17

On 07/18/17 06:33, Alex Henrie wrote: > @@ -2421,7 +2421,6 @@ double CDECL MSVCR120_expm1(double x) > #else > double ret = exp(x) - 1; > #endif > - if (!isfinite(ret)) *MSVCRT__errno() = MSVCRT_ERANGE; > return ret; > } The function behaves differently depending on the reason of overflow. It doesn't set ERANGE errno if x == INFINITY (as your tests show). But it does set it when e.g. x == 10e100. Thanks, Piotr

1 0

Re: [PATCH 5/5] ucrtbase/tests: Add tests for errno with math functions.
by Piotr Caban 18 Jul '17

18 Jul '17

On 07/18/17 06:33, Alex Henrie wrote: > +#define INF 1.0/0.0 Please define infinity in the same way as it's done in other tests. You can use __port_infinity or call p_strtod("inf"...) to obtain it. Thanks, Piotr

1 0

Re: [PATCH 2/8] regedit: Determine the number of values and subkeys before exporting any data
by Alexandre Julliard 18 Jul '17

18 Jul '17

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > programs/regedit/regproc.c | 212 +++++++++++++++++++-------------------------- > 1 file changed, 91 insertions(+), 121 deletions(-) This doesn't seem to be an improvement. The registry can change while you are exporting it, you can't rely on querying things beforehand. -- Alexandre Julliard julliard(a)winehq.org

2 2