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

Re: [APPDB] query.php: implement query_affected_rows()
by Sebastian Lackner 14 May '17

14 May '17

On 15.05.2017 03:13, Rosanne DiMesio wrote: > +function query_affected_rows(); Does this change work as expected? The ";" at the end of the function definition looks suspicious.

2 1

Re: [PATCH] dwrite/tests: Fix rendering mode variable type (Coverity)
by Sebastian Lackner 14 May '17

14 May '17

On 14.05.2017 22:49, Nikolay Sivov wrote: > - mode1 = IDWriteRenderingParams3_GetRenderingMode(params3); > - ok(mode1 == DWRITE_RENDERING_MODE1_NATURAL_SYMMETRIC, "got %d\n", mode1); > + mode = IDWriteRenderingParams3_GetRenderingMode(params3); > + ok(mode == DWRITE_RENDERING_MODE_NATURAL_SYMMETRIC, "got %d\n", mode1); ^^^^^ What about the remaining occurrence of mode1?

1 0

Re: [v7 PATCH] oleaut32: Improve support for IDispatch in VarCat
by Marvin 11 May '17

11 May '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=30935 Your paranoid android. === wxppro (32 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === w2003std (32 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === wvistau64 (32 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === w2008s64 (32 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === w7u (32 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === w7pro64 (32 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === w8 (32 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === w864 (32 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === w1064 (32 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === wvistau64 (64 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === w2008s64 (64 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === w7pro64 (64 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === w864 (64 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e === w1064 (64 bit vartest) === vartest.c:6153: Test failed: got 0x8007000e

1 0

Re: [v6 PATCH] oleaut32: Improve support for IDispatch in VarCat
by Sebastian Lackner 11 May '17

11 May '17

On 12.05.2017 02:11, Alistair Leslie-Hughes wrote: > +static HRESULT WINAPI DummyDispatch_Invoke(IDispatch *iface, > + DISPID dispid, REFIID riid, > + LCID lcid, WORD wFlags, > + DISPPARAMS *params, > + VARIANT *res, > + EXCEPINFO *ei, > + UINT *arg_err) > +{ [...] > + if (FAILED(This->result)) > + return FAILED(This->result); I didn't review the whole patch, but this return value looks suspicious for a HRESULT function.

1 0

Re: [PATCH 8/9] d3dx9: Add test for 'cmp' preshader opcode.
by Matteo Bruni 11 May '17

11 May '17

2017-05-08 17:39 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/tests/effect.c | 33 +++++++++++++++++++++++++++++---- > 1 file changed, 29 insertions(+), 4 deletions(-) > > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index 015b505..dfc3f5b 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -4490,7 +4490,8 @@ static void test_preshader_op(IDirect3DDevice9 *device, const DWORD *sample_effe > { > {0, 0, 0}, > {5549, 0, 4}, > - {5400, 2, 1} > + {5400, 2, 1}, > + {4121, 0, 1} > }; You could add a ',' after the new array element so that, if we ever need to add another test, that line doesn't need to be changed. Just mentioning it since you have to resend anyway. > DWORD *test_effect_blob; > HRESULT hr; > @@ -4506,8 +4507,30 @@ static void test_preshader_op(IDirect3DDevice9 *device, const DWORD *sample_effe > > test_effect_blob = HeapAlloc(GetProcessHeap(), 0, sample_effect_blob_size); > memcpy(test_effect_blob, sample_effect_blob, sample_effect_blob_size); > - for (i = 0; i < blob_position[test->args_count].ins_count; ++i) > - test_effect_blob[op_pos + i * op_step] = test->opcode; > + > + if (test->args_count == 3) > + { > + DWORD *pos; > + > + pos = &test_effect_blob[op_pos - 1]; > + *pos++ = 1; > + *pos++ = test->opcode; > + *pos++ = test->args_count; > + for (i = 0; i < 3; ++i) > + { > + *pos++ = 0; > + *pos++ = 2; > + *pos++ = i == 2 ? 4 : 0; > + } > + *pos++ = 0; > + *pos++ = 4; > + *pos++ = 0; > + } > + else > + { > + for (i = 0; i < blob_position[test->args_count].ins_count; ++i) > + test_effect_blob[op_pos + i * op_step] = test->opcode; > + } I think this is getting more and more ugly. What about writing an effect with those 3 preshaders by hand instead? You could start with those edited effect fragments, fixing them up and putting them together in a new effect blob. Sprinkle over some comments as necessary. I'm not going to block the patch for this reason but I think it's something worth exploring.

2 15

Re: [APPDB] admin.php: fix incorrect call to query_affected_rows
by Bruno Jesus 11 May '17

11 May '17

On Thu, May 11, 2017 at 3:27 PM, Rosanne DiMesio <dimesio(a)earthlink.net> wrote: > Should be query_num_rows. > > Signed-off-by: Rosanne DiMesio <dimesio(a)earthlink.net> > --- > admin.php | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/admin.php b/admin.php > index 09c0aca..fe1ccc6 100644 > --- a/admin.php > +++ b/admin.php > @@ -199,7 +199,7 @@ function fixNoteLinks() > > echo " Deleting note links "; > $hResult = query_parameters("DELETE FROM appNotes WHERE linkedWith != '0'"); > - echo "Deleted ".query_affected_rows()." links "; > + echo "Deleted ".query_num_rows()." links "; Is this some particularity of the wrapper database function? I say that because for example both mysql and postgresql have the 2 distinct functions where affected_rows is used for delete/update and num_rows is used for select queries.

1 0

Re: [APPDB] admin.php: fix incorrect call to query_affected_rows
by Michael Müller 11 May '17

11 May '17

Am 11.05.2017 um 20:27 schrieb Rosanne DiMesio: > $hResult = query_parameters("DELETE FROM appNotes WHERE linkedWith != '0'"); > - echo "Deleted ".query_affected_rows()." links "; > + echo "Deleted ".query_num_rows()." links "; I suspect the new code is as broken as the old one. The function query_num_rows returns the number of query results, which is always zero for a DELETE query. The correct solution would be to implement query_affected_rows() based on the corresponding php function [1]. [1]: http://php.net/manual/de/mysqli.affected-rows.php

1 0

Re: [PATCH] winemac.drv: Implement systray version 1-3 notifications.
by Ken Thomases 11 May '17

11 May '17

On May 11, 2017, at 6:15 AM, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > > Signed-off-by: Stefan Dösinger <stefan(a)codeweavers.com> > > --- > > I am not sure how to handle NOTIFY_VERSION_4. The click events do not contain > a coordinate and I don't know how to read the position of the icon. The first > issue is probably trivial to handle by assuming a fixed coordinate because the > icon isn't all that big anyway. The latter is a larger problem because a wrong > coordinate could cause an app to show a popup in the wrong place. Also on dual > screen setups there are two copies of the status bar and two icon copies. Pick > the wrong one and things go bad as well. Thanks. I've sent a modified version that properly handles version 4. The internal events that you were looking at didn't contain the position, but can be made to. :) -Ken

1 0

Re: [PATCH] oleaut32: Improve support for IDispatch in VarCat
by Jacek Caban 11 May '17

11 May '17

Hi Alistair, On 17.04.2017 05:43, Alistair Leslie-Hughes wrote: > diff --git a/dlls/oleaut32/variant.c b/dlls/oleaut32/variant.c > index 17f753af70..6f695c3f37 100644 > --- a/dlls/oleaut32/variant.c > +++ b/dlls/oleaut32/variant.c > @@ -2611,6 +2611,13 @@ HRESULT WINAPI VarCat(LPVARIANT left, LPVARIANT right, LPVARIANT out) > V_VT(&bstrvar_left) = VT_BSTR; > V_BSTR(&bstrvar_left) = SysAllocString(sz_empty); > } > + else if (rightvt == VT_DISPATCH) > + { > + /* The returned Dispatch Value, may not be able to be coerced, ie. VT_NULL */ > + hres = VariantChangeTypeEx(&bstrvar_left,left,0,0,VT_BSTR); > + if (hres != S_OK && hres != DISP_E_TYPEMISMATCH) > + return hres; > + } > else > { > hres = VariantChangeTypeEx(&bstrvar_left,left,0,0,VT_BSTR); > @@ -2651,6 +2658,13 @@ HRESULT WINAPI VarCat(LPVARIANT left, LPVARIANT right, LPVARIANT out) > V_VT(&bstrvar_right) = VT_BSTR; > V_BSTR(&bstrvar_right) = SysAllocString(sz_empty); > } > + else if (rightvt == VT_DISPATCH) > + { > + /* The returned Dispatch Value, may not be able to be coerced, ie. VT_NULL */ > + hres = VariantChangeTypeEx(&bstrvar_right,right,0,0,VT_BSTR); > + if (hres != S_OK && hres != DISP_E_TYPEMISMATCH) > + return hres; > + } > else > { > hres = VariantChangeTypeEx(&bstrvar_right,right,0,0,VT_BSTR); While this change may do the right thing in the case you're fixing, the whole surrounding code looks just wrong and I don't like the idea of making it even more complicated to work around problem in this single case. For example take a look at if() statement in line 2620. It's never executed, because it requires leftvt to be VT_NULL, which is handled elsewhere (line 2609). If you remove that if(), this call is almost identical to what you add here as a special case. The only difference is DISP_E_TYPEMISMATCH handling. Did you investigate what happens in such case? Maybe we should handle VariantChangeTypeEx failures like you do in the patch? Thanks, Jacek

1 0

Re: [PATCH 3/9] d3dx9: Add 'asin' preshader opcode.
by Matteo Bruni 11 May '17

11 May '17

2017-05-08 17:39 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +/* is_nan() does not work (by design) if compiled with gcc -ffinite-math-only option which is implied by > + * -ffast_math. */ > +static BOOL pres_is_nan(double v) > +{ > + return (*(ULONG64 *)&v & 0x7ff8000000000000) == 0x7ff8000000000000; > +} I don't think we care about people compiling Wine with broken CFLAGS. We should just trust the isnan() library function. FWIW we already do so in other places in Wine.

2 3