November 2016 - Wine-Devel - list.winehq.org

Re: [PATCH 1/2] regedit: Overhaul Unicode line processing
by Alexandre Julliard Nov. 22, 2016

Nov. 22, 2016

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > + p = strpbrkW(line, line_endings); > + if (!p) > + { > + HeapFree(GetProcessHeap(), 0, buf); > + return NULL; If there are no line endings you need to read more data instead of failing. Also it looks to me that you are reading the entire file into the buffer instead of just one line. -- Alexandre Julliard julliard(a)winehq.org

2 4

Code style guidelines on the wiki
by Alex Henrie Nov. 22, 2016

Nov. 22, 2016

As requested at WineConf, I have rewritten the wiki's notes about code style to match our current practices, as best I understand them: https://wiki.winehq.org/Submitting_Patches#Code_guidelines Please read this over, let me know what you think, and feel free to edit the wiki directly. -Alex

2 1

readelf: Error: the dynamic segment offset + size exceeds the size of the file
by Hin-Tak Leung Nov. 22, 2016

Nov. 22, 2016

Hi, Just rebuilding the wine staging rpm from fedora, and towards the end I see piles of "readelf: Error: the dynamic segment offset + size exceeds the size of the file" on wine's dll.so's. This is specific to the rpm build and packaging process - AFAIK this strips off the debuginfo so that it can be shipped as a separate package. Nonetheless, should I worry about it, and/or file this as a problem of readelf (binutils)? Thanks. Hin-Tak

1 0

Re: [4/5] quartz: Add tests for IBasicVideo.
by Andrew Eikum Nov. 21, 2016

Nov. 21, 2016

It's a good start, though I'm a little concerned your tests didn't turn up the bugs I found in 2/5, 3/5, and 5/5, nor the bug I noticed in BaseControlVideoImpl_get_DestinationHeight while reviewing your patches... Andrew On Sun, Nov 20, 2016 at 03:33:25PM +0900, Akihiro Sagawa wrote: > Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com> > --- > dlls/quartz/tests/filtergraph.c | 93 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 93 insertions(+) > > diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c > index 87b1123..a92aab3 100644 > --- a/dlls/quartz/tests/filtergraph.c > +++ b/dlls/quartz/tests/filtergraph.c > @@ -50,6 +50,97 @@ static int createfiltergraph(void) > &CLSID_FilterGraph, NULL, CLSCTX_INPROC_SERVER, &IID_IGraphBuilder, (LPVOID*)&pgraph); > } > > +static void test_basic_video(void) > +{ > + IBasicVideo* pbv; > + LONG video_width, video_height; > + LONG left, top, width, height; > + HRESULT hr; > + > + hr = IGraphBuilder_QueryInterface(pgraph, &IID_IBasicVideo, (LPVOID*)&pbv); > + ok(hr==S_OK, "Cannot get IBasicVideo interface returned: %x\n", hr); > + > + /* test get video size */ > + hr = IBasicVideo_GetVideoSize(pbv, NULL, NULL); > + ok(hr==E_POINTER, "IBasicVideo_GetVideoSize returned: %x\n", hr); > + hr = IBasicVideo_GetVideoSize(pbv, &video_width, NULL); > + ok(hr==E_POINTER, "IBasicVideo_GetVideoSize returned: %x\n", hr); > + hr = IBasicVideo_GetVideoSize(pbv, NULL, &video_height); > + ok(hr==E_POINTER, "IBasicVideo_GetVideoSize returned: %x\n", hr); > + hr = IBasicVideo_GetVideoSize(pbv, &video_width, &video_height); > + ok(hr==S_OK, "Cannot get video size returned: %x\n", hr); > + > + /* test source position */ > + hr = IBasicVideo_GetSourcePosition(pbv, NULL, NULL, NULL, NULL); > + ok(hr == E_POINTER, "IBasicVideo_GetSourcePosition returned: %x\n", hr); > + hr = IBasicVideo_GetSourcePosition(pbv, &left, &top, NULL, NULL); > + ok(hr == E_POINTER, "IBasicVideo_GetSourcePosition returned: %x\n", hr); > + hr = IBasicVideo_GetSourcePosition(pbv, NULL, NULL, &width, &height); > + ok(hr == E_POINTER, "IBasicVideo_GetSourcePosition returned: %x\n", hr); > + hr = IBasicVideo_GetSourcePosition(pbv, &left, &top, &width, &height); > + ok(hr == S_OK, "Cannot get source position returned: %x\n", hr); > + ok(left == 0, "expected 0, got %d\n", left); > + ok(top == 0, "expected 0, got %d\n", top); > + ok(width == video_width, "expected %d, got %d\n", video_width, width); > + ok(height == video_height, "expected %d, got %d\n", video_height, height); > + > + hr = IBasicVideo_SetSourcePosition(pbv, 0, 0, 0, 0); > + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr); > + hr = IBasicVideo_put_SourceTop(pbv, -1); > + ok(hr==E_INVALIDARG, "IBasicVideo_put_SourceTop returned: %x\n", hr); > + hr = IBasicVideo_put_SourceTop(pbv, 0); > + ok(hr==S_OK, "Cannot put source top returned: %x\n", hr); > + hr = IBasicVideo_put_SourceTop(pbv, 1); > + todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_put_SourceTop returned: %x\n", hr); > + > + hr = IBasicVideo_SetSourcePosition(pbv, video_width, 0, 1, video_height); > + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr); > + hr = IBasicVideo_SetSourcePosition(pbv, 0, video_height, video_width, 1); > + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr); > + > + hr = IBasicVideo_SetSourcePosition(pbv, 0, 0, video_width, video_height+1); > + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr); > + hr = IBasicVideo_SetSourcePosition(pbv, 0, 0, video_width+1, video_height); > + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr); > + > + /* test destination position */ > + hr = IBasicVideo_GetDestinationPosition(pbv, NULL, NULL, NULL, NULL); > + ok(hr == E_POINTER, "IBasicVideo_GetDestinationPosition returned: %x\n", hr); > + hr = IBasicVideo_GetDestinationPosition(pbv, &left, &top, NULL, NULL); > + ok(hr == E_POINTER, "IBasicVideo_GetDestinationPosition returned: %x\n", hr); > + hr = IBasicVideo_GetDestinationPosition(pbv, NULL, NULL, &width, &height); > + ok(hr == E_POINTER, "IBasicVideo_GetDestinationPosition returned: %x\n", hr); > + hr = IBasicVideo_GetDestinationPosition(pbv, &left, &top, &width, &height); > + ok(hr == S_OK, "Cannot get destination position returned: %x\n", hr); > + ok(left == 0, "expected 0, got %d\n", left); > + ok(top == 0, "expected 0, got %d\n", top); > + todo_wine ok(width == video_width, "expected %d, got %d\n", video_width, width); > + todo_wine ok(height == video_height, "expected %d, got %d\n", video_height, height); > + > + hr = IBasicVideo_SetDestinationPosition(pbv, 0, 0, 0, 0); > + ok(hr==E_INVALIDARG, "IBasicVideo_SetDestinationPosition returned: %x\n", hr); > + > + hr = IBasicVideo_put_DestinationLeft(pbv, -1); > + todo_wine ok(hr==S_OK, "Cannot put destination left returned: %x\n", hr); > + hr = IBasicVideo_put_DestinationLeft(pbv, 0); > + todo_wine ok(hr==S_OK, "Cannot put destination left returned: %x\n", hr); > + hr = IBasicVideo_put_DestinationLeft(pbv, 1); > + todo_wine ok(hr==S_OK, "Cannot put destination left returned: %x\n", hr); > + > + hr = IBasicVideo_SetDestinationPosition(pbv, 0, 0, video_width * 2, video_height * 2); > + ok(hr==S_OK, "Cannot set destination position returned: %x\n", hr); > + > + /* reset source position */ > + hr = IBasicVideo_SetSourcePosition(pbv, 0, 0, video_width, video_height); > + ok(hr==S_OK, "Cannot set source position returned: %x\n", hr); > + > + /* reset destination position */ > + hr = IBasicVideo_SetDestinationPosition(pbv, 0, 0, video_width, video_height); > + ok(hr==S_OK, "Cannot set destination position returned: %x\n", hr); > + > + IBasicVideo_Release(pbv); > +} > + > static void rungraph(void) > { > HRESULT hr; > @@ -70,6 +161,8 @@ static void rungraph(void) > > IMediaFilter_Release(pmf); > > + test_basic_video(); > + > hr = IMediaControl_Run(pmc); > ok(hr==S_FALSE, "Cannot run the graph returned: %x\n", hr); > >

1 0

Re: [3/5] strmbase: Add validation checks when updating destination rectangle.
by Andrew Eikum Nov. 21, 2016

Nov. 21, 2016

On Sun, Nov 20, 2016 at 03:33:24PM +0900, Akihiro Sagawa wrote: > @@ -73,6 +73,17 @@ static HRESULT BaseControlVideoImpl_CheckSourceRect(BaseControlVideo *This, RECT > return S_OK; > } > > +static HRESULT BaseControlVideoImpl_CheckTargetRect(BaseControlVideo *This, RECT *pTargetRect) > +{ > + if (pTargetRect->top >= pTargetRect->bottom || > + pTargetRect->left >= pTargetRect->right || > + (pTargetRect->bottom - pTargetRect->top) <= 0 || > + (pTargetRect->right - pTargetRect->left) <= 0) > + return E_INVALIDARG; > + The third and fourth conditions seem redundant with the first and second. You could use IsRectEmpty() here as well. Andrew

1 0

Re: [5/5] strmbase: Shift source/destination rectangle when updating top or left property.
by Andrew Eikum Nov. 21, 2016

Nov. 21, 2016

On Sun, Nov 20, 2016 at 03:33:26PM +0900, Akihiro Sagawa wrote: > @@ -212,7 +212,7 @@ HRESULT WINAPI BaseControlVideoImpl_put_SourceLeft(IBasicVideo *iface, LONG Sour > hr = This->pFuncsTable->pfnGetSourceRect(This, &SourceRect); > if (SUCCEEDED(hr)) > { > - SourceRect.left = SourceLeft; > + OffsetRect(&SourceRect, SourceLeft, 0); > hr = BaseControlVideoImpl_CheckSourceRect(This, &SourceRect); > } > if (SUCCEEDED(hr)) These look wrong to me. The SourceLeft argument is an absolute position, while OffsetRect takes an offset. Check my math, but I think you want something like OffsetRect( SourceLeft - SourceRect.left ). Andrew

1 0

Re: [3/5] strmbase: Add validation checks when updating destination rectangle.
by Andrew Eikum Nov. 21, 2016

Nov. 21, 2016

On Sun, Nov 20, 2016 at 03:33:24PM +0900, Akihiro Sagawa wrote: > @@ -409,13 +438,19 @@ HRESULT WINAPI BaseControlVideoImpl_put_DestinationHeight(IBasicVideo *iface, LO > { > RECT DestRect; > BaseControlVideo *This = impl_from_IBasicVideo(iface); > + HRESULT hr; > > TRACE("(%p/%p)->(%d)\n", This, iface, DestinationHeight); > - This->pFuncsTable->pfnGetTargetRect(This, &DestRect); > - DestRect.right = DestRect.left + DestinationHeight; > - This->pFuncsTable->pfnSetTargetRect(This, &DestRect); > + hr = This->pFuncsTable->pfnGetTargetRect(This, &DestRect); > + if (SUCCEEDED(hr)) > + { > + DestRect.right = DestRect.left + DestinationHeight; You've got a copy-paste error here, unfortunately. Andrew

1 0

Re: [PATCH 6/6] d3d9/tests: Add test for occlusion query with result bigger than UINT32_MAX.
by Marvin Nov. 21, 2016

Nov. 21, 2016

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=26773 Your paranoid android. === w864 (64 bit device) === device.c:4392: Test failed: cw is 0xf60, expected 0x7f. device.c:4401: Test failed: Callback cw is 0xf60, expected 0x7f. === w1064 (64 bit device) === device.c:4392: Test failed: cw is 0xf60, expected 0x7f. device.c:4401: Test failed: Callback cw is 0xf60, expected 0x7f.

1 0

Re: [PATCH v2] winemapi: Directly use xdg-email if available, enabling file attachments.
by Hans Leidekker Nov. 21, 2016

Nov. 21, 2016

On Thu, 2016-11-17 at 13:43 -0600, Jeremy White wrote: > +static ULONG add_argument(char **argv, int *argc, const char *arg, const char *param) > +{ > + argv[(*argc)] = HeapAlloc(GetProcessHeap(), 0, strlen(arg) + 1); > + if (!argv[(*argc)]) > + return MAPI_E_INSUFFICIENT_MEMORY; > + strcpy(argv[(*argc)++], arg); > + > + if (param) > + { > + argv[(*argc)] = HeapAlloc(GetProcessHeap(), 0, strlen(param) + 1); > + if (!argv[(*argc)]) > + return MAPI_E_INSUFFICIENT_MEMORY; > + strcpy(argv[(*argc)++], param); > + } > + > + return SUCCESS_SUCCESS; This still leaks when the first allocation succeeds and the second fails. > +static ULONG add_file(char **argv, int *argc, const char *path, const char *file) > +{ > + WCHAR *fullname, *p; > + char *unixpath; > + int namelen = 1; > + ULONG ret; > + > + if (path) > + namelen += strlen(path) + 1; > + if (file) > + namelen += strlen(file); > + > + p = fullname = HeapAlloc(GetProcessHeap(), 0, namelen * sizeof(WCHAR)); > + if (!fullname) > + return MAPI_E_INSUFFICIENT_MEMORY; > + memset(fullname, 0, namelen * sizeof(WCHAR)); You should pass HEAP_ZERO_MEMORY to HeapAlloc if you need a zero-initialized block but in this case it's sufficient to initialize the first element of fullname. > + if (path) > + { > + MultiByteToWideChar(CP_ACP, 0, path, -1, p, namelen); > + p += strlen(path); > + MultiByteToWideChar(CP_ACP, 0, "\\", 1, p, 1); > + p++; This is shorter and doesn't make an assumption about conversion length: p += MultiByteToWideChar(CP_ACP, 0, path, -1, p, namelen); > +ULONG XDGSendMail(LHANDLE session, ULONG_PTR uiparam, > + lpMapiMessage message, FLAGS flags, ULONG reserved) > +{ > + int i; > + int argc = 0; > + int max_args; > + char **argv = NULL; > + ULONG ret; > + > + TRACE("(0x%08lx 0x%08lx %p 0x%08x 0x%08x)\n", session, uiparam, message, flags, reserved); > + > + if (!message) > + return MAPI_E_FAILURE; > + > + max_args = 1 + (2 + message->nRecipCount + message->nFileCount) * 2; > + argv = HeapAlloc(GetProcessHeap(), 0, (max_args + 1) * sizeof(*argv)); > + if (!argv) > + return MAPI_E_INSUFFICIENT_MEMORY; > + > + memset(argv, 0, (max_args + 1) * sizeof(*argv)); See above.

1 0

Crash after accessing parts before allocated memory
by Fabian Maurer Nov. 21, 2016

Nov. 21, 2016

This is a kind-of follow up to "Debugging wine and finding the cause of a crash", but another question. The program I'm debugging works reliably on windows/ReactOS, but on wine it crashes every time. I can get it to work by hacking RtlAllocateHeap to always allocate 5500 bytes more than needed, but that's hardly a solution. >From what I currently know, the issue stems from a few negative offsets being added on a pointer returned from RtlAllocateHeap. On windows this leads to accessing a memory region filled with zeroes, on wine it accesses random memory. The program can handle nullpointers, but not bogus pointer. Any ideas how to deal with that, or is UB like that just a no-go? I have zero knowledge about the internal management of the heap, so help would be appreciated. At least is seems suspicious subtracting something from a pointer it got from RtlAllocateHeap, but maybe there's some plan behind it.

3 4