November 2015 - Wine-devel - list.winehq.org

Re: [PATCH] user32: Allow WM_CAPTURECHANGE indicating SetCapture() parameter.
by Dmitry Timoshkov 24 Nov '15

24 Nov '15

Christopher Thielen <cthielen(a)gmail.com> wrote: > +// Not const in order to set lParam during test Using c++ comment in Wine code is forbidden. Also it should not be needed to test lparam, everything is clear enough, and I'd suggest to make the structure 'const'. Looks good to me otherwise. -- Dmitry.

2 4

Re: ole32: Print error message when IPersistStorage_Load fails (Coverity)
by Nikolay Sivov 23 Nov '15

23 Nov '15

On 23.11.2015 22:26, André Hentschel wrote: > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > CID 1336207 > > dlls/ole32/compobj.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c > index 2f55076..b146376 100644 > --- a/dlls/ole32/compobj.c > +++ b/dlls/ole32/compobj.c > @@ -3443,7 +3443,9 @@ HRESULT WINAPI CoGetInstanceFromIStorage( > > if (ps) > { > - IPersistStorage_Load(ps, storage); > + hr = IPersistStorage_Load(ps, storage); > + if (FAILED(hr)) > + ERR("failed to load storage\n"); > IPersistStorage_Release(ps); > } > > The question is what to do on this failure, that's a point of Coverity warning.

2 1

Re: msacm32/tests: Add some acmStreamPrepareHeader and acmStreamConvert tests
by Marvin 23 Nov '15

23 Nov '15

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=18454 Your paranoid android. === wvistau64 (64 bit msacm) === msacm.c:621: Test failed: prepare failed: 0xb msacm.c:622: Test failed: header wasn't prepared: 0x0 msacm.c:628: Test failed: convert failed: 0x202 msacm.c:629: Test failed: conversion was not done: 0x0 msacm.c:630: Test failed: expected 512, got 12345 msacm.c:632: Test failed: expected 1010, got 12345 msacm.c:635: Test failed: unprepare failed: 0x202 msacm.c:636: Test failed: header wasn't unprepared: 0x0 === w2008s64 (64 bit msacm) === msacm.c:621: Test failed: prepare failed: 0xb msacm.c:622: Test failed: header wasn't prepared: 0x0 msacm.c:628: Test failed: convert failed: 0x202 msacm.c:629: Test failed: conversion was not done: 0x0 msacm.c:630: Test failed: expected 512, got 12345 msacm.c:632: Test failed: expected 1010, got 12345 msacm.c:635: Test failed: unprepare failed: 0x202 msacm.c:636: Test failed: header wasn't unprepared: 0x0 === w7pro64 (64 bit msacm) === msacm.c:621: Test failed: prepare failed: 0xb msacm.c:622: Test failed: header wasn't prepared: 0x0 msacm.c:628: Test failed: convert failed: 0x202 msacm.c:629: Test failed: conversion was not done: 0x0 msacm.c:630: Test failed: expected 512, got 12345 msacm.c:632: Test failed: expected 1010, got 12345 msacm.c:635: Test failed: unprepare failed: 0x202 msacm.c:636: Test failed: header wasn't unprepared: 0x0 === w864 (64 bit msacm) === msacm.c:621: Test failed: prepare failed: 0xb msacm.c:622: Test failed: header wasn't prepared: 0x0 msacm.c:628: Test failed: convert failed: 0x202 msacm.c:629: Test failed: conversion was not done: 0x0 msacm.c:630: Test failed: expected 512, got 12345 msacm.c:632: Test failed: expected 1010, got 12345 msacm.c:635: Test failed: unprepare failed: 0x202 msacm.c:636: Test failed: header wasn't unprepared: 0x0 === w1064 (64 bit msacm) === msacm.c:621: Test failed: prepare failed: 0xb msacm.c:622: Test failed: header wasn't prepared: 0x0 msacm.c:628: Test failed: convert failed: 0x202 msacm.c:629: Test failed: conversion was not done: 0x0 msacm.c:630: Test failed: expected 512, got 12345 msacm.c:632: Test failed: expected 1010, got 12345 msacm.c:635: Test failed: unprepare failed: 0x202 msacm.c:636: Test failed: header wasn't unprepared: 0x0

2 1

question about deferred patches
by Jamie Taylor 23 Nov '15

23 Nov '15

I have a question about the patch management system. The description of the "deferred" status is: > This means that the patch is deferred because of code freeze for an > upcoming release. You need to either resend after the release, make > the patch less intrusive, or make a convincing argument that it needs > to be included in the release. Why do patches have to be resubmitted after a release rather than just marking all deferred patches as "New" after the release? Thanks, Jamie Taylor

3 2

Re: [PATCH] user32: Allow WM_CAPTURECHANGE indicating SetCapture() parameter.
by Marvin 23 Nov '15

23 Nov '15

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=18446 Your paranoid android. === w1064 (32 bit msg) === msg.c:6094: Test failed: Update region shouldn't be empty msg.c:6328: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6330: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6100: Test failed: Regions are different msg.c:6100: Test failed: Regions are different msg.c:6100: Test failed: Regions are different

1 0

Re: msacm32: Avoid attempts to convert data when available output is negative in acmStreamConvert
by Andrew Eikum 23 Nov '15

23 Nov '15

This doesn't seem right to me. The test succeeds even without your changes to msacm32/stream.c because cbDstLength != cbPreparedDstLength in acmStreamConvert. I tried moving cbDstLength=-4 before PrepareHeader, and both PrepareHeader and Convert succeed on my Win7 VM. Can you look further into this? Andrew On Sun, Nov 22, 2015 at 11:46:33PM +0800, Bruno Jesus wrote: > Signed-off-by: Bruno Jesus <00cpxxx(a)gmail.com> > > The game Lost Horizon passes a negative value in cbDstLength while > calling acmStreamConvert, this means we can't do the conversion > because there is no output buffer available. Cast required since the > value is DWORD. > > Fixes https://bugs.winehq.org/show_bug.cgi?id=24723 > diff --git a/dlls/msacm32/stream.c b/dlls/msacm32/stream.c > index bdfc3cc..67c5ce9 100644 > --- a/dlls/msacm32/stream.c > +++ b/dlls/msacm32/stream.c > @@ -88,20 +88,25 @@ MMRESULT WINAPI acmStreamConvert(HACMSTREAM has, PACMSTREAMHEADER pash, > > if ((was = ACM_GetStream(has)) == NULL) { > WARN("invalid handle\n"); > - return MMSYSERR_INVALHANDLE; > + return MMSYSERR_INVALHANDLE; > } > if (!pash || pash->cbStruct < sizeof(ACMSTREAMHEADER)) { > WARN("invalid parameter\n"); > - return MMSYSERR_INVALPARAM; > + return MMSYSERR_INVALPARAM; > } > if (!(pash->fdwStatus & ACMSTREAMHEADER_STATUSF_PREPARED)) { > WARN("unprepared header\n"); > - return ACMERR_UNPREPARED; > + return ACMERR_UNPREPARED; > } > > pash->cbSrcLengthUsed = 0; > pash->cbDstLengthUsed = 0; > > + if ((LONG) pash->cbDstLength < 0) { > + WARN("invalid output length\n"); > + return MMSYSERR_INVALPARAM; > + } > + > /* Note: the ACMSTREAMHEADER and ACMDRVSTREAMHEADER structs are of same > * size. some fields are private to msacm internals, and are exposed > * in ACMSTREAMHEADER in the dwReservedDriver array > diff --git a/dlls/msacm32/tests/msacm.c b/dlls/msacm32/tests/msacm.c > index 4c9dca0..3b5a446 100644 > --- a/dlls/msacm32/tests/msacm.c > +++ b/dlls/msacm32/tests/msacm.c > @@ -527,7 +527,7 @@ static void test_prepareheader(void) > WAVEFORMATEX dst; > MMRESULT mr; > ACMSTREAMHEADER hdr; > - BYTE buf[sizeof(WAVEFORMATEX) + 32], pcm[512], input[512]; > + BYTE buf[sizeof(WAVEFORMATEX) + 32], pcm[2048], input[512]; > ADPCMCOEFSET *coef; > > src = (ADPCMWAVEFORMAT*)buf; > @@ -567,6 +567,7 @@ static void test_prepareheader(void) > mr = acmStreamOpen(&has, NULL, (WAVEFORMATEX*)src, &dst, NULL, 0, 0, 0); > ok(mr == MMSYSERR_NOERROR, "open failed: 0x%x\n", mr); > > + memset(input, 0, sizeof(input)); > memset(&hdr, 0, sizeof(hdr)); > hdr.cbStruct = sizeof(hdr); > hdr.pbSrc = input; > @@ -594,10 +595,37 @@ static void test_prepareheader(void) > ok(mr == MMSYSERR_NOERROR, "prepare failed: 0x%x\n", mr); > ok(hdr.fdwStatus == (ACMSTREAMHEADER_STATUSF_PREPARED | ACMSTREAMHEADER_STATUSF_DONE), "header wasn't prepared: 0x%x\n", hdr.fdwStatus); > > + hdr.fdwStatus &= ~ACMSTREAMHEADER_STATUSF_DONE; > + mr = acmStreamConvert(has, &hdr, ACM_STREAMCONVERTF_BLOCKALIGN); > + ok(mr == MMSYSERR_NOERROR, "convert failed: 0x%x\n", mr); > + ok(hdr.fdwStatus & ACMSTREAMHEADER_STATUSF_DONE, "conversion was not done: 0x%x\n", hdr.fdwStatus); > + > mr = acmStreamUnprepareHeader(has, &hdr, 0); > ok(mr == MMSYSERR_NOERROR, "unprepare failed: 0x%x\n", mr); > ok(hdr.fdwStatus == ACMSTREAMHEADER_STATUSF_DONE, "header wasn't unprepared: 0x%x\n", hdr.fdwStatus); > > + memset(&hdr, 0, sizeof(hdr)); > + hdr.cbStruct = sizeof(hdr); > + hdr.pbSrc = input; > + hdr.cbSrcLength = sizeof(input); > + hdr.pbDst = pcm; > + hdr.cbDstLength = sizeof(pcm); > + > + mr = acmStreamPrepareHeader(has, &hdr, 0); > + ok(mr == MMSYSERR_NOERROR, "prepare failed: 0x%x\n", mr); > + ok(hdr.fdwStatus == ACMSTREAMHEADER_STATUSF_PREPARED, "header wasn't prepared: 0x%x\n", hdr.fdwStatus); > + > + hdr.cbDstLength = -4; /* Test for Lost Horizon (bug 24723) */ > + hdr.cbDstLengthUsed = 12345; > + mr = acmStreamConvert(has, &hdr, ACM_STREAMCONVERTF_BLOCKALIGN); > + ok(mr == MMSYSERR_INVALPARAM, "expected 11, got %d!\n", mr); > + ok(hdr.cbDstLengthUsed == 0, "expected 0, got %d\n", hdr.cbDstLengthUsed); > + > + hdr.cbDstLength = sizeof(pcm); > + mr = acmStreamUnprepareHeader(has, &hdr, 0); > + ok(mr == MMSYSERR_NOERROR, "unprepare failed: 0x%x\n", mr); > + ok(hdr.fdwStatus == 0, "header wasn't unprepared: 0x%x\n", hdr.fdwStatus); > + > mr = acmStreamClose(has, 0); > ok(mr == MMSYSERR_NOERROR, "close failed: 0x%x\n", mr); > }

2 1

Re: msvcp90: basic_string_wchar_dtor should return NULL on i386.
by Sebastian Lackner 23 Nov '15

23 Nov '15

On 23.11.2015 13:33, Piotr Caban wrote: > Probably the commit message would be more readable if it says: > msvcp90: basic_string_wchar_dtor should set EAX to 0 on i386. EAX is used to store the return value, so both commit messages are basically equivalent. If your suggested subject is preferred, I have no problem when its changed while applying the patch or to resend later. > > According to my knowledge the application depends on that. > > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > > > >

2 1

Re: [PATCH 2/2] comctl32: Item with negative cChildren acts as if it actually had children
by Alexandre Julliard 23 Nov '15

23 Nov '15

Nikolay Sivov <nsivov(a)codeweavers.com> writes: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/comctl32/tests/treeview.c | 22 +++++++++++++++++++++- > dlls/comctl32/treeview.c | 5 +++-- > 2 files changed, 24 insertions(+), 3 deletions(-) What application is this fixing? -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: msi: Use the intended attributes when overwriting the target file in cabinet_copy_file.
by Hans Leidekker 23 Nov '15

23 Nov '15

On Mon, 2015-11-23 at 14:13 +0100, Thomas Faber wrote: > The current code uses the old file's attributes, which means it would > make the file read-only again. This would e.g. prevent the deletion of > the file at the end of test_readonlyfile_cab (and should probably even > make this CreateFileW call fail since it specifies GENERIC_WRITE). > > The reason this currently works in Wine is that CreateFile keeps the > existing file attributes in this case (see the todo_wine in > test_NtCreateFile). > > The patch fixes msi to use the correct attributes as returned by the > callback. Looks good but if it doesn't fix a real installer I think we should hold off until the freeze is over.

1 0

Re: [PATCH 01/3] msvcp110: Add tr2_sys__Read_dir implementation.(try 3)
by Piotr Caban 23 Nov '15

23 Nov '15

Hi, the patches looks good for me. Since they are not fixing any regression/don't make application running I think you will need to send them after code freeze. Thanks, Piotr

1 0