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

Re: [PATCH 2/5] d3d11/tests: Add test for vertex formats.

by Marvin

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=24112 Your paranoid android. === build (build) === Patch failed to apply

July 8, 2016

Re: [3/3] wininet/tests: Add a test for recently introduced regression related to InternetCrackUrl.

by Marvin

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=24111 Your paranoid android. === build (build) === Patch failed to apply

July 8, 2016

Re: [2/3] wininet/tests: Avoid unnecessary initialization of variables.

by Marvin

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=24110 Your paranoid android. === build (build) === Patch failed to apply

July 8, 2016

Re: [1/3] wininet: Correctly set lpszUrlPath in InternetCrackUrlW for urls without path component.

by Marvin

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=24109 Your paranoid android. === build (build) === Patch failed to apply

July 8, 2016

Re: gdi32: Remove unused function.

by Huw Davies

On Fri, Jul 08, 2016 at 03:58:04PM +0200, Sebastian Lackner wrote: > I assume it was intentional to remove the is_in_rect() check from PtInRegion, but > is it really such a good idea? For points outside of the extents rectangle it > increased runtime from O(1) to O(log N), which could still matter in practice. Good catch. I hadn't meant to remove the extents test which is why I didn't remove is_in_rect() in the original patch. Thanks, Huw.

July 8, 2016

Re: gdi32: Remove unused function.

by Sebastian Lackner

On 08.07.2016 12:25, Huw Davies wrote: > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > --- > dlls/gdi32/region.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/dlls/gdi32/region.c b/dlls/gdi32/region.c > index 0b0ff47..1e10221 100644 > --- a/dlls/gdi32/region.c > +++ b/dlls/gdi32/region.c > @@ -149,11 +149,6 @@ static inline void empty_region( WINEREGION *reg ) > reg->extents.left = reg->extents.top = reg->extents.right = reg->extents.bottom = 0; > } > > -static inline BOOL is_in_rect( const RECT *rect, int x, int y ) > -{ > - return (rect->right > x && rect->left <= x && rect->bottom > y && rect->top <= y); > -} > - > /* > * number of points to buffer before sending them off > * to scanlines() : Must be an even number > I assume it was intentional to remove the is_in_rect() check from PtInRegion, but is it really such a good idea? For points outside of the extents rectangle it increased runtime from O(1) to O(log N), which could still matter in practice.

July 8, 2016

Re: gdi32: Optionally return the type from GDI_GetObjPtr().

by Huw Davies

On Fri, Jul 08, 2016 at 08:28:56PM +0900, Alexandre Julliard wrote: > The performance impact is negligible, but it doesn't make a lot of sense > to return the type when it was already specified by the caller. Some > kind of get_any_obj_ptr(HGDIOBJ, WORD *) function for the any type case > would probably be cleaner. Hmm, ok. Yes, that makes sense. New version sent. Huw.

July 8, 2016

Re: gdi32: Optionally return the type from GDI_GetObjPtr().

by Alexandre Julliard

Huw Davies <huw(a)codeweavers.com> writes: > On Fri, Jul 08, 2016 at 05:24:29PM +0800, Dmitry Timoshkov wrote: >> Huw Davies <huw(a)codeweavers.com> wrote: >> >> > This enables get_dc_obj() to check the type without calling GetObjectType() >> > and thus it saves additional calls to Enter/LeaveCriticalSection(). >> > >> > Signed-off-by: Huw Davies <huw(a)codeweavers.com> >> > --- >> > This reduces the runtime of the dib tests by about 8% and achieves a similar >> > increase in performance with the drawing phase of a (gdi heavy) app I'm >> > looking at. >> ... >> > - bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP ); >> > + bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP, NULL ); >> >> Did you consider introducing GDI_GetObjPtrEx() with additional pointer to >> type, and making a shortcut by defining/calling it by a not Ex version? > > No. > >> That would simplify the patch and avoid penalizing/changing the callers >> that don't need the type. > > We don't don't need to retain backwards compatibility so I don't see > the point, indeed an additional 'Ex' version would be more > confusing IMO. Almost all of the time in that function is spent > grabbing the crit sec, so any time saved by not filling in the out > param is totally negligible. The performance impact is negligible, but it doesn't make a lot of sense to return the type when it was already specified by the caller. Some kind of get_any_obj_ptr(HGDIOBJ, WORD *) function for the any type case would probably be cleaner. -- Alexandre Julliard julliard(a)winehq.org

July 8, 2016

Re: [v3] riched20: Add support for ECO_NOHIDESEL flag

by Huw Davies

On Fri, Jul 08, 2016 at 12:32:55PM +0200, Piotr Caban wrote: > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/riched20/editor.c | 9 +++++++-- > dlls/riched20/paint.c | 6 ++++-- > 2 files changed, 11 insertions(+), 4 deletions(-) > > > diff --git a/dlls/riched20/editor.c b/dlls/riched20/editor.c > index 13a2857..cb6fe3d 100644 > --- a/dlls/riched20/editor.c > +++ b/dlls/riched20/editor.c > @@ -3453,14 +3453,15 @@ LRESULT ME_HandleMessage(ME_TextEditor *editor, UINT msg, WPARAM wParam, > ME_RewrapRepaint(editor); > } > > + if ((changedSettings & settings & ECO_NOHIDESEL) && !editor->bHaveFocus) > + ME_InvalidateSelection( editor ); > + Actually on reflection, I think comparing with ES_NOHIDESEL is slightly less confusing, so let's go with v2. Huw.

July 8, 2016

Re: riched20: Add support for ECO_NOHIDESEL flag

by Huw Davies

On Fri, Jul 08, 2016 at 12:33:41PM +0200, Piotr Caban wrote: > On 07/08/16 12:29, Piotr Caban wrote: > >On 07/08/16 12:11, Huw Davies wrote: > >>I think this, and all subsequent ECO_NOHIDESELs, would be better as > >>ES_NOHIDESEL. > >>The first one is fine as ECO_NOHIDESEL, since that's the setting one. > >I've sent a fixed version. I'm not sure if I have changed it as you wanted. > I guess v3 is what you were asking about. It was, but let's go with v2, on reflection it's a bit less confusing. Huw.

July 8, 2016

Re: riched20: Add support for ECO_NOHIDESEL flag

by Piotr Caban

On 07/08/16 12:29, Piotr Caban wrote: > On 07/08/16 12:11, Huw Davies wrote: >> I think this, and all subsequent ECO_NOHIDESELs, would be better as >> ES_NOHIDESEL. >> The first one is fine as ECO_NOHIDESEL, since that's the setting one. > I've sent a fixed version. I'm not sure if I have changed it as you wanted. I guess v3 is what you were asking about. Thanks, Piotr

July 8, 2016

Re: riched20: Add support for ECO_NOHIDESEL flag

by Piotr Caban

On 07/08/16 12:11, Huw Davies wrote: > I think this, and all subsequent ECO_NOHIDESELs, would be better as ES_NOHIDESEL. > The first one is fine as ECO_NOHIDESEL, since that's the setting one. I've sent a fixed version. I'm not sure if I have changed it as you wanted. Thanks, Piotr

July 8, 2016

Re: riched20: Add support for ECO_NOHIDESEL flag

by Huw Davies

On Fri, Jul 08, 2016 at 11:34:11AM +0200, Piotr Caban wrote: > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > > This patch also fixes control behavior when ECO_NOHIDESEL is not specified > and the control looses focus (for bug 40915). > --- > dlls/riched20/editor.c | 9 +++++++-- > dlls/riched20/paint.c | 6 ++++-- > 2 files changed, 11 insertions(+), 4 deletions(-) > > > diff --git a/dlls/riched20/editor.c b/dlls/riched20/editor.c > index 13a2857..b7864eb 100644 > --- a/dlls/riched20/editor.c > +++ b/dlls/riched20/editor.c > @@ -3453,14 +3453,15 @@ LRESULT ME_HandleMessage(ME_TextEditor *editor, UINT msg, WPARAM wParam, > ME_RewrapRepaint(editor); > } > > + if ((changedSettings & settings & ECO_NOHIDESEL) && !editor->bHaveFocus) > + ME_InvalidateSelection( editor ); > + > if (changedSettings & settings & ECO_VERTICAL) > FIXME("ECO_VERTICAL not implemented yet!\n"); > if (changedSettings & settings & ECO_AUTOHSCROLL) > FIXME("ECO_AUTOHSCROLL not implemented yet!\n"); > if (changedSettings & settings & ECO_AUTOVSCROLL) > FIXME("ECO_AUTOVSCROLL not implemented yet!\n"); > - if (changedSettings & settings & ECO_NOHIDESEL) > - FIXME("ECO_NOHIDESEL not implemented yet!\n"); > if (changedSettings & settings & ECO_WANTRETURN) > FIXME("ECO_WANTRETURN not implemented yet!\n"); > if (changedSettings & settings & ECO_AUTOWORDSELECTION) > @@ -4257,6 +4258,8 @@ LRESULT ME_HandleMessage(ME_TextEditor *editor, UINT msg, WPARAM wParam, > editor->bHaveFocus = TRUE; > ME_ShowCaret(editor); > ME_SendOldNotify(editor, EN_SETFOCUS); > + if (!editor->bHideSelection && !(editor->styleFlags & ECO_NOHIDESEL)) I think this, and all subsequent ECO_NOHIDESELs, would be better as ES_NOHIDESEL. The first one is fine as ECO_NOHIDESEL, since that's the setting one. Huw.

July 8, 2016

Re: gdi32: Optionally return the type from GDI_GetObjPtr().

by Huw Davies

On Fri, Jul 08, 2016 at 05:24:29PM +0800, Dmitry Timoshkov wrote: > Huw Davies <huw(a)codeweavers.com> wrote: > > > This enables get_dc_obj() to check the type without calling GetObjectType() > > and thus it saves additional calls to Enter/LeaveCriticalSection(). > > > > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > > --- > > This reduces the runtime of the dib tests by about 8% and achieves a similar > > increase in performance with the drawing phase of a (gdi heavy) app I'm > > looking at. > ... > > - bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP ); > > + bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP, NULL ); > > Did you consider introducing GDI_GetObjPtrEx() with additional pointer to > type, and making a shortcut by defining/calling it by a not Ex version? No. > That would simplify the patch and avoid penalizing/changing the callers > that don't need the type. We don't don't need to retain backwards compatibility so I don't see the point, indeed an additional 'Ex' version would be more confusing IMO. Almost all of the time in that function is spent grabbing the crit sec, so any time saved by not filling in the out param is totally negligible. Huw.

July 8, 2016

Re: gdi32: Optionally return the type from GDI_GetObjPtr().

by Dmitry Timoshkov

Huw Davies <huw(a)codeweavers.com> wrote: > This enables get_dc_obj() to check the type without calling GetObjectType() > and thus it saves additional calls to Enter/LeaveCriticalSection(). > > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > --- > This reduces the runtime of the dib tests by about 8% and achieves a similar > increase in performance with the drawing phase of a (gdi heavy) app I'm > looking at. ... > - bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP ); > + bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP, NULL ); Did you consider introducing GDI_GetObjPtrEx() with additional pointer to type, and making a shortcut by defining/calling it by a not Ex version? That would simplify the patch and avoid penalizing/changing the callers that don't need the type. -- Dmitry.

July 8, 2016

Re: [PATCH 02/12] d3drm: Move d3drm_device struct to d3drm_private.h.

by Aaryaman Vasishta

> > +struct d3drm_device > +{ > + IDirect3DRMDevice IDirect3DRMDevice_iface; > + IDirect3DRMDevice2 IDirect3DRMDevice2_iface; > + IDirect3DRMDevice3 IDirect3DRMDevice3_iface; > + IDirect3DRMWinDevice IDirect3DRMWinDevice_iface; > + IDirect3DRM *d3drm; > + IDirectDraw *ddraw; > + IDirectDrawSurface *primary_surface, *render_target; > + IDirectDrawClipper *clipper; > + IDirect3DDevice *device; > + LONG ref; > + BOOL dither; > + D3DRMRENDERQUALITY quality; > + DWORD rendermode; > + DWORD height; > + DWORD width; > +}; > + > > I should remove the current forward declaration now that this patch moves the definition here.

July 8, 2016

Re: [PATCH 04/12] d3drm: Implement IDirect3DRMDevice*::Add/DeleteDestroyCallback.

by Aaryaman Vasishta

On Thu, Jul 7, 2016 at 7:22 PM, Aaryaman Vasishta <jem456.vasishta(a)gmail.com > wrote: > > @@ -65,6 +65,7 @@ void d3drm_device_destroy(struct d3drm_device *device) > IDirectDraw_Release(device->ddraw); > IDirect3DRM_Release(device->d3drm); > } > + d3drm_object_cleanup((IDirect3DRMObject > *)&device->IDirect3DRMDevice_iface, &device->obj); > HeapFree(GetProcessHeap(), 0, device); > } > I should've probably put d3drm_object_cleanup at the beginning.

July 8, 2016

Re: [PATCH v2 2/3] winex11.drv: avoid passing NULL to memcpy (clang)

by Kirill Smirnov

According to the standard, memcpy() and its friends should never accept NULL as argument, otherwise the behavior is undefined and optimizer takes this into account. Below in the same file (line 206) caller passes NULL as lpComp argument triggering this case. This patch is of the same nature as the following: commit 5a3ad0ecf08c00fc0b7b2f24e55be27b7c3f5da0 Author: Kirill Smirnov <kirill.k.smirnov(a)gmail.com> Date: Mon May 9 18:56:14 2016 +0300 winhlp32: Do not pass NULL to strchr() (spotted by clang). Signed-off-by: Kirill K. Smirnov <kirill.k.smirnov(a)gmail.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> On 08.07.2016 06:16, Alexandre Julliard wrote: > Kirill Smirnov <kirill.k.smirnov(a)gmail.com> writes: > >> @@ -92,7 +92,8 @@ static void X11DRV_ImmSetInternalString(DWORD dwOffset, >> ptr_new = CompositionString + byte_offset; >> memmove(ptr_new + byte_length, ptr_new + byte_selection, >> dwCompStringLength - byte_offset - byte_selection); >> - memcpy(ptr_new, lpComp, byte_length); >> + /* Do not pass NULL even if the length == 0 */ >> + if (lpComp) memcpy(ptr_new, lpComp, byte_length); >> dwCompStringLength += byte_expansion; > > Why would that be a problem? >

July 8, 2016

Re: [PATCH v2 3/3] winex11.drv: fix NULL pointer dereference (clang)

by Alexandre Julliard

Kirill Smirnov <kirill.k.smirnov(a)gmail.com> writes: > @@ -1679,6 +1679,11 @@ static void xrender_put_image( Pixmap src_pixmap, Picture src_pict, Picture mask > } > else > { > + if (!physdev) > + { > + ERR("physdev is NULL\n"); > + return; > + } This should never happen. -- Alexandre Julliard julliard(a)winehq.org

July 7, 2016

Re: [PATCH v2 2/3] winex11.drv: avoid passing NULL to memcpy (clang)

by Alexandre Julliard

Kirill Smirnov <kirill.k.smirnov(a)gmail.com> writes: > @@ -92,7 +92,8 @@ static void X11DRV_ImmSetInternalString(DWORD dwOffset, > ptr_new = CompositionString + byte_offset; > memmove(ptr_new + byte_length, ptr_new + byte_selection, > dwCompStringLength - byte_offset - byte_selection); > - memcpy(ptr_new, lpComp, byte_length); > + /* Do not pass NULL even if the length == 0 */ > + if (lpComp) memcpy(ptr_new, lpComp, byte_length); > dwCompStringLength += byte_expansion; Why would that be a problem? -- Alexandre Julliard julliard(a)winehq.org

July 7, 2016

Re: [PATCH] server: Move the \BaseNamedObjects symlink from \Sessions\0 to \Sessions\1\BaseNamedObjects.

by Sebastian Lackner

On 08.07.2016 03:05, flan wrote: > On Fri, Jul 08, 2016 at 01:17:01AM +0200, Sebastian Lackner wrote: >> I'm not the maintainer of this component, but Alex is right that you >> will have to use your real name (both as author and also in the >> Signed-off-by message). >> >> I also believe that the variable names link_bno1W and link_bno1_str >> are chosen a bit badly. The 1 does not really make any sense, and instead >> of "bno" it might be better to use the same abbreviation "basenamed" as >> above. If you make any changes, please also adjust the whitespace >> inbetween of the variable name and "=" to match the surrounding code. >> >> If \\Sessions\\1 contains all the elements from \\, you could also >> think about symlinking it to root (instead of symlinking each component >> separately). >> >> Regards, >> Sebastian >> > > Thank you for the feedback, I'll change variable names and adjust the > whitespace. \\Sessions\\1 does not seem to contain all elements from \\. The > only element besides BaseNamedObjects that's both in \\ and \\Sessions\\1 > seems to be \\Windows and its child directory, WindowStations. Okay, then the current approach should be fine. > > Is there any particular reason behind the real name requirement? > For the Wine project it is very important to make sure that all developers are trustworthy and that they do not violate any laws, for example by disassembling or looking at original Microsoft code. You can find more about the requirements and guidelines on the following pages: https://wiki.winehq.org/Submitting_Patches https://wiki.winehq.org/Clean_Room_Guidelines Although your patch is just a simple fix, for reasons of simplicity the rules are the same for all kind of submissions.

July 7, 2016

Re: [PATCH] server: Move the \BaseNamedObjects symlink from \Sessions\0 to \Sessions\1\BaseNamedObjects.

by flan

On Fri, Jul 08, 2016 at 01:17:01AM +0200, Sebastian Lackner wrote: > I'm not the maintainer of this component, but Alex is right that you > will have to use your real name (both as author and also in the > Signed-off-by message). > > I also believe that the variable names link_bno1W and link_bno1_str > are chosen a bit badly. The 1 does not really make any sense, and instead > of "bno" it might be better to use the same abbreviation "basenamed" as > above. If you make any changes, please also adjust the whitespace > inbetween of the variable name and "=" to match the surrounding code. > > If \\Sessions\\1 contains all the elements from \\, you could also > think about symlinking it to root (instead of symlinking each component > separately). > > Regards, > Sebastian > Thank you for the feedback, I'll change variable names and adjust the whitespace. \\Sessions\\1 does not seem to contain all elements from \\. The only element besides BaseNamedObjects that's both in \\ and \\Sessions\\1 seems to be \\Windows and its child directory, WindowStations. Is there any particular reason behind the real name requirement?

July 7, 2016

Re: [PATCH] server: Move the \BaseNamedObjects symlink from \Sessions\0 to \Sessions\1\BaseNamedObjects.

by Alex Henrie

2016-07-07 17:31 GMT-06:00 Sebastian Lackner <sebastian(a)fds-team.de>: > On 08.07.2016 01:29, Alex Henrie wrote: >> 2016-07-07 17:17 GMT-06:00 Sebastian Lackner <sebastian(a)fds-team.de>: >>> I'm not the maintainer of this component >> >> MAINTAINERS seems to say that you're in charge of everything under >> server/ - if that's not the case, it should be updated to be more >> specific. > > I fear you are mixing up the meaning of M and P. ;) > > M: Primary maintainer. CC patches to: FullName <address(a)domain> > P: Other knowledgeable person. FullName <address(a)domain> You're right, sorry. I'll pay more attention to that in the future. -Alex

July 7, 2016

Re: [PATCH] server: Move the \BaseNamedObjects symlink from \Sessions\0 to \Sessions\1\BaseNamedObjects.

by Sebastian Lackner

On 08.07.2016 01:29, Alex Henrie wrote: > 2016-07-07 17:17 GMT-06:00 Sebastian Lackner <sebastian(a)fds-team.de>: >> I'm not the maintainer of this component > > MAINTAINERS seems to say that you're in charge of everything under > server/ - if that's not the case, it should be updated to be more > specific. I fear you are mixing up the meaning of M and P. ;) M: Primary maintainer. CC patches to: FullName <address(a)domain> P: Other knowledgeable person. FullName <address(a)domain> > > Thanks for giving Flan some suggestions. > > -Alex >

July 7, 2016

Re: [PATCH] server: Move the \BaseNamedObjects symlink from \Sessions\0 to \Sessions\1\BaseNamedObjects.

by Alex Henrie

2016-07-07 17:17 GMT-06:00 Sebastian Lackner <sebastian(a)fds-team.de>: > I'm not the maintainer of this component MAINTAINERS seems to say that you're in charge of everything under server/ - if that's not the case, it should be updated to be more specific. Thanks for giving Flan some suggestions. -Alex

July 7, 2016

Re: [PATCH] server: Move the \BaseNamedObjects symlink from \Sessions\0 to \Sessions\1\BaseNamedObjects.

by Sebastian Lackner

On 07.07.2016 23:59, Alex Henrie wrote: > Hi Flan, > > Thanks for your interest in contributing to Wine. The Wine project > requires you to use your real name (both first and last name) when > submitting patches. It might also be a good idea to contact Sebastian > Lackner, who is listed as a maintainer of this part of Wine. He might > be able to give you more specific feedback, or he might decide to > include your patch in Wine Staging to test it with a wider audience. > > -Alex > I'm not the maintainer of this component, but Alex is right that you will have to use your real name (both as author and also in the Signed-off-by message). I also believe that the variable names link_bno1W and link_bno1_str are chosen a bit badly. The 1 does not really make any sense, and instead of "bno" it might be better to use the same abbreviation "basenamed" as above. If you make any changes, please also adjust the whitespace inbetween of the variable name and "=" to match the surrounding code. If \\Sessions\\1 contains all the elements from \\, you could also think about symlinking it to root (instead of symlinking each component separately). Regards, Sebastian

July 7, 2016

Re: [PATCH v2] kernel32: Use setprogname(3) to set the program name if available.

by Ken Thomases

On Apr 26, 2016, at 3:38 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > > On 26.04.2016 22:35, Theodore Dubois wrote: >> I have both a custom wine loader and a hacked version of Wine, so I just reverted the patch. >> >> It would be really nice to have this work for when I’m not using a custom loader. Maybe you could check if argv is in the malloc heap? Or maybe I could somehow indicate when calling wine_init that the argv is malloced? >> >> ~Theodore >> > > May I ask why you are not just allocating the argv array in a way compatible to the OS? > When you have full control in your custom loader, this should be trivial and the easiest > way to fix it. Forgive me for reviving an old thread, but how was it determined that allocating all of the strings pointed to by argv as a single buffer is what the OS does? This does not seem like a portable assumption. Also, why should set_process_name() shift the strings up within the (supposedly contiguous) string buffer? For the HAVE_SETPROCTITLE, it doesn't do that. Prior to Chip's commit, it didn't do that on macOS. So, Theodore's original question, "Why wouldn’t setprogname(argv[1]) be adequate?", is pretty close. It should have been, "Why wouldn’t setprogname(argv[1]) followed by shifting the elements of argv up one be adequate?" I'm not even sure why the string buffer is shifted for Linux. It looks like it's leftover logic that's no longer relevant (having to do with vdm?). I'll also note that Chip's commit broke things for the case where none of HAVE_SETPROCTITLE, HAVE_SETPROGNAME, or HAVE_PRCTL is defined. In that case, nothing shifts the elements of argv up one anymore. It used to. -Ken

July 7, 2016

Re: [PATCH] server: Move the \BaseNamedObjects symlink from \Sessions\0 to \Sessions\1\BaseNamedObjects.

by Alex Henrie

Hi Flan, Thanks for your interest in contributing to Wine. The Wine project requires you to use your real name (both first and last name) when submitting patches. It might also be a good idea to contact Sebastian Lackner, who is listed as a maintainer of this part of Wine. He might be able to give you more specific feedback, or he might decide to include your patch in Wine Staging to test it with a wider audience. -Alex

July 7, 2016

[PATCH] server: Move the \BaseNamedObjects symlink from \Sessions\0 to \Sessions\1\BaseNamedObjects.

by flan

What is wrong with the patch? Right now the most glaring formatting issue I can find is inconsistent spacing in a variable declaration, but I'm asking just in case there are more problems I haven't noticed.

July 7, 2016

[PATCH] server: Move the \BaseNamedObjects symlink from \Sessions\0 to \Sessions\1\BaseNamedObjects.

by flan

What is wrong with the patch? Right now the most glaring formatting issue I can find is inconsistent spacing in a variable declaration, but I'm asking just in case there are more problems I haven't noticed.

July 7, 2016

Re: [PATCH 01/12] d3drm: Store reference to IDirect3DRM in struct d3drm_device.

by Aaryaman Vasishta

The end of the subject should have "d3drm_device_create" instead of "struct d3drm_device". On Thu, Jul 7, 2016 at 8:18 PM, Marvin <testbot(a)winehq.org> wrote: > Thank you for your contribution to Wine! > > This is an automated notification to let you know that your patch has > been reviewed and its status set to "Assigned". > > This means that the patch has been assigned to a reviewer. If the > reviewer signs off on the patch, it will be committed, otherwise > you'll receive advice on how to improve it. If you don't hear anything > after a couple of weeks, please resend the patch. >

July 7, 2016

Re: [PATCH] wininet: fix the bug can not save proxy bypass server into regedit by ie internet options dialog

by Alex Henrie

Hi huangxuewei, Thanks for your interest in contributing to Wine! The Wine project requires you to use your real name (with capitalization and spaces) when submitting patches. It would also help to link to a bug report in the patch email, and to add tests to demonstrate that the behavior you want is the correct behavior. -Alex

July 7, 2016

Re: [PATCH] ws2_32: Allow user to enable IP dual stack (v4)

by Marvin

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=24060 Your paranoid android. === wvistau64 (32 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: -559038737; 10048 expected if IPv6 binds to IPv4 as well). === w2008s64 (32 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: -559038737; 10048 expected if IPv6 binds to IPv4 as well). sock: unhandled exception c0000005 at 00000000 === w7u (32 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === w7pro64 (32 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === w8 (32 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === w864 (32 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === w1064 (32 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === wvistau64 (64 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: -559038737; 10048 expected if IPv6 binds to IPv4 as well). === w2008s64 (64 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: -559038737; 10048 expected if IPv6 binds to IPv4 as well). === w7pro64 (64 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === w864 (64 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === w1064 (64 bit sock) === sock.c:6149: Test failed: IPV6_V6ONLY is enabled after bind. sock.c:6158: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well).

July 7, 2016

Re: [PATCH 3/3] mshtml: Export IElementSelector for all elements in compatibility mode >= IE8.

by Marvin

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=24059 Your paranoid android. === wxppro (32 bit) === Found no trace of the test summary line or of a crash === w2003std (32 bit) === Found no trace of the test summary line or of a crash === wvistau64 (32 bit) === Found no trace of the test summary line or of a crash === w2008s64 (32 bit) === Found no trace of the test summary line or of a crash === w7u (32 bit) === Found no trace of the test summary line or of a crash === w7pro64 (32 bit) === Found no trace of the test summary line or of a crash === w8 (32 bit) === Found no trace of the test summary line or of a crash === w864 (32 bit) === Found no trace of the test summary line or of a crash === w1064 (32 bit) === Found no trace of the test summary line or of a crash === wvistau64 (64 bit) === Found no trace of the test summary line or of a crash === w2008s64 (64 bit) === Found no trace of the test summary line or of a crash === w7pro64 (64 bit) === Found no trace of the test summary line or of a crash === w864 (64 bit) === Found no trace of the test summary line or of a crash === w1064 (64 bit) === Found no trace of the test summary line or of a crash

July 7, 2016

Re: [10/10] webservices: Implement WsWriteBody.

by Marvin

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=24055 Your paranoid android. === build (build) === Make failed

July 7, 2016

Re: [09/10] webservices: Implement WsWriteEnvelopeEnd.

by Marvin

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=24054 Your paranoid android. === build (build) === Make failed

July 7, 2016

Re: [08/10] webservices: Implement WsWriteEnvelopeStart.

by Marvin

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=24053 Your paranoid android. === build (build) === Make failed

July 7, 2016

Re: [07/10] webservices: Implement WsAddressMessage.

by Marvin

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=24052 Your paranoid android. === build (build) === Make failed

July 7, 2016

Re: [06/10] webservices: Implement WsInitializeMessage.

by Marvin

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=24051 Your paranoid android. === build (build) === Make failed

July 7, 2016

Re: [05/10] webservices: Implement WsCreateMessageForChannel.

by Marvin

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=24050 Your paranoid android. === build (build) === Make failed

July 7, 2016

Re: [03/10] webservices: Allow integer values to be passed by pointer in WsWriteType.

by Marvin

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=24049 Your paranoid android. === build (build) === Make failed

July 7, 2016

Re: [02/10] webservices: Inherit the prefix only if parent and child namespaces match.

by Marvin

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=24048 Your paranoid android. === build (build) === Make failed

July 7, 2016

Re: [01/10] webservices: Add support for writing double values.

by Marvin

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=24047 Your paranoid android. === build (build) === Make failed

July 7, 2016

Re: [PATCH v3 6/6] shell32: Set output interfaces to NULL in FolderItems functions.

by Alex Henrie

2016-07-06 11:39 GMT-06:00 Alex Henrie <alexhenrie24(a)gmail.com>: > 2016-07-06 1:13 GMT-06:00 Michael Stefaniuc <mstefani(a)redhat.com>: >> On 07/06/2016 06:55 AM, Alex Henrie wrote: >>> [...] >> >> This two if's look odd: one for ppid and then for !ppid. >> Why don't you just unconditionally do the NULL assignment after the >> check below? >> >>> if (!ppid) >>> return E_INVALIDARG; > > Thanks for catching that. This is a mistake, according to the tests > get_Parent should never return E_INVALIDARG. The only difference between v3 and v4 of the patchset is the removal of these two lines from FolderItemsImpl_get_Parent. -Alex

July 7, 2016

Re: [2/2] kernel32/tests: Add a test for sorting of InMemoryOrderModuleList.

by Sebastian Lackner

On 07.07.2016 06:54, Marvin wrote: > 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=24042 > > Your paranoid android. > > > === w7u (32 bit loader) === > loader.c:2308: Test failed: WaitForSingleObject failed: 102 > loader.c:2310: Test failed: expected exit code 199, got 259 > loader.c:2329: Test failed: expected 198, got 199 > > === w1064 (32 bit loader) === > loader.c:2733: Test failed: Test 0: ResolveDelayLoadedAPI failed > loader.c:2734: Test failed: Test 0: expected 74374550, got 00000000 > loader.c:2733: Test failed: Test 1: ResolveDelayLoadedAPI failed > loader.c:2734: Test failed: Test 1: expected 72BC2CF0, got 00000000 > loader.c:2733: Test failed: Test 2: ResolveDelayLoadedAPI failed > loader.c:2734: Test failed: Test 2: expected 74372DC0, got 00000000 > loader.c:2741: Test failed: Test 3: ResolveDelayLoadedAPI succeeded with 00000000 > loader.c:2742: Test failed: Test 3: Wrong callback count: 0 > loader.c:2741: Test failed: Test 4: ResolveDelayLoadedAPI succeeded with 00000000 > loader.c:2742: Test failed: Test 4: Wrong callback count: 0 > loader.c:1820: Test failed: expected STILL_ACTIVE, got 195 > loader.c:1855: Test failed: expected STILL_ACTIVE, got 195 > > === w864 (64 bit loader) === > loader.c:1820: Test failed: expected STILL_ACTIVE, got 195 > loader.c:1855: Test failed: expected STILL_ACTIVE, got 195 > > === w1064 (64 bit loader) === > loader.c:2733: Test failed: Test 0: ResolveDelayLoadedAPI failed > loader.c:2734: Test failed: Test 0: expected 00007FFC76DA5510, got 0000000000000000 > loader.c:2733: Test failed: Test 1: ResolveDelayLoadedAPI failed > loader.c:2734: Test failed: Test 1: expected 00007FFC6EFE1B00, got 0000000000000000 > loader.c:2733: Test failed: Test 2: ResolveDelayLoadedAPI failed > loader.c:2734: Test failed: Test 2: expected 00007FFC76DA2390, got 0000000000000000 > loader.c:2741: Test failed: Test 3: ResolveDelayLoadedAPI succeeded with 0000000000000000 > loader.c:2742: Test failed: Test 3: Wrong callback count: 0 > loader.c:2741: Test failed: Test 4: ResolveDelayLoadedAPI succeeded with 0000000000000000 > loader.c:2742: Test failed: Test 4: Wrong callback count: 0 > loader.c:1820: Test failed: expected STILL_ACTIVE, got 195 > loader.c:1855: Test failed: expected STILL_ACTIVE, got 195 > These are all existing failures.

July 6, 2016

Re: [2/2] kernel32/tests: Add a test for sorting of InMemoryOrderModuleList.

by Marvin

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=24042 Your paranoid android. === w7u (32 bit loader) === loader.c:2308: Test failed: WaitForSingleObject failed: 102 loader.c:2310: Test failed: expected exit code 199, got 259 loader.c:2329: Test failed: expected 198, got 199 === w1064 (32 bit loader) === loader.c:2733: Test failed: Test 0: ResolveDelayLoadedAPI failed loader.c:2734: Test failed: Test 0: expected 74374550, got 00000000 loader.c:2733: Test failed: Test 1: ResolveDelayLoadedAPI failed loader.c:2734: Test failed: Test 1: expected 72BC2CF0, got 00000000 loader.c:2733: Test failed: Test 2: ResolveDelayLoadedAPI failed loader.c:2734: Test failed: Test 2: expected 74372DC0, got 00000000 loader.c:2741: Test failed: Test 3: ResolveDelayLoadedAPI succeeded with 00000000 loader.c:2742: Test failed: Test 3: Wrong callback count: 0 loader.c:2741: Test failed: Test 4: ResolveDelayLoadedAPI succeeded with 00000000 loader.c:2742: Test failed: Test 4: Wrong callback count: 0 loader.c:1820: Test failed: expected STILL_ACTIVE, got 195 loader.c:1855: Test failed: expected STILL_ACTIVE, got 195 === w864 (64 bit loader) === loader.c:1820: Test failed: expected STILL_ACTIVE, got 195 loader.c:1855: Test failed: expected STILL_ACTIVE, got 195 === w1064 (64 bit loader) === loader.c:2733: Test failed: Test 0: ResolveDelayLoadedAPI failed loader.c:2734: Test failed: Test 0: expected 00007FFC76DA5510, got 0000000000000000 loader.c:2733: Test failed: Test 1: ResolveDelayLoadedAPI failed loader.c:2734: Test failed: Test 1: expected 00007FFC6EFE1B00, got 0000000000000000 loader.c:2733: Test failed: Test 2: ResolveDelayLoadedAPI failed loader.c:2734: Test failed: Test 2: expected 00007FFC76DA2390, got 0000000000000000 loader.c:2741: Test failed: Test 3: ResolveDelayLoadedAPI succeeded with 0000000000000000 loader.c:2742: Test failed: Test 3: Wrong callback count: 0 loader.c:2741: Test failed: Test 4: ResolveDelayLoadedAPI succeeded with 0000000000000000 loader.c:2742: Test failed: Test 4: Wrong callback count: 0 loader.c:1820: Test failed: expected STILL_ACTIVE, got 195 loader.c:1855: Test failed: expected STILL_ACTIVE, got 195

July 6, 2016

Re: [PATCH 1/3] kernel32/tests/fiber: Add tests for fiber-local storage.

by John Sheu

Thanks! I'm interested in using fiber-local storage, particularly for the callback behavior (which is not implemented right now). The Tls* API in Windows does not support this. This is for the purposes of better thread-local storage for the Ion library: see https://github.com/google/ion On Sat, Jul 2, 2016 at 7:55 AM, André Hentschel <nerv(a)dawncrow.de> wrote: > Am 02.07.2016 um 04:41 schrieb John Sheu: >> Signed-off-by: John Sheu <sheu(a)google.com> > > Hi and welcome to Wine development! > May I ask if you could name an application which will benefit from your fixes? > Your tests currently fail on our testbot as you can see here: https://source.winehq.org/patches/ > Some inline comments: > >> static VOID WINAPI FiberMainProc(LPVOID lpFiberParameter) >> { >> BYTE *tparam = (BYTE *)lpFiberParameter; >> - cbCount++; >> + fiberCount++; >> ok(*tparam == 185, "Parameterdata expected not to be changed\n"); >> + if (fls_index_to_set != FLS_OUT_OF_INDEXES) >> + { >> + if (!pFlsSetValue || !pFlsGetValue) >> + { >> + win_skip( "Fiber Local Storage not supported\n" ); >> + } >> + else >> + { >> + PVOID pret; >> + BOOL bret; >> + >> + pret = FlsGetValue(fls_index_to_set); >> + ok(pret == NULL, "FlsGetValue returned %p, expected NULL\n", pret); >> + >> + /* Set the FLS value */ >> + SetLastError( 0xdeadbeef ); >> + bret = pFlsSetValue(fls_index_to_set, fls_value_to_set); >> + ok(bret, "FlsSetValue failed with error %u\n", GetLastError()); >> + >> + /* Verify that FlsGetValue retrieves the value set by FlsSetValue */ >> + SetLastError( 0xdeadbeef ); >> + pret = FlsGetValue(fls_index_to_set); >> + ok(pret == fls_value_to_set, >> + "FlsGetValue returned %p, expected %p\n", pret, fls_value_to_set); >> + ok(GetLastError() == ERROR_SUCCESS, >> + "FlsGetValue error %u\n", GetLastError()); >> + } >> + } >> pSwitchToFiber(fibers[0]); >> } > > You are mixing tabs and spaces in FiberMainProc, spaces are preferred. Further you can avoid some line brakes in ok() calls, only wrap at around 100 > >> +static void test_FiberLocalStorageWithFibers(PFLS_CALLBACK_FUNCTION cbfunc) { >> + PVOID val1 = (PVOID) 0x314; >> + PVOID val2 = (PVOID) 0x152; >> + BOOL ret; >> + >> + if (!pConvertThreadToFiber || !pSwitchToFiber || !pDeleteFiber) >> + { >> + win_skip( "Fibers not supported\n" ); >> + return; >> + } >> + if (!pFlsAlloc || !pFlsFree || !pFlsSetValue || !pFlsGetValue) >> + { >> + win_skip( "Fiber Local Storage not supported\n" ); >> + return; >> + } >> + >> + >> + fls_index_to_set = pFlsAlloc(cbfunc); >> + ok(fls_index_to_set != FLS_OUT_OF_INDEXES, "FlsAlloc failed with error %d\n", GetLastError()); >> + >> + fiberCount = 0; >> + cbCount = 0; >> + fibers[0] = pConvertThreadToFiber(&testparam); > > Why do you convert the current thread into a fiber? It seems to me that is causing the crash on w2k3. > You could try to run that test in a seperate thread > >> + fibers[1] = pCreateFiber(0,FiberMainProc,&testparam); >> + fibers[2] = pCreateFiber(0,FiberMainProc,&testparam); >> + ok(fibers[1] != 0, "CreateFiber failed with error %d\n", GetLastError()); >> + ok(fibers[2] != 0, "CreateFiber failed with error %d\n", GetLastError()); >> + ok(fiberCount == 0, "Wrong fiber count: %d\n", fiberCount); >> + ok(cbCount == 0, "Wrong callback count: %d\n", cbCount); >> + >> + fiberCount = 0; >> + cbCount = 0; >> + fls_value_to_set = val1; >> + pSwitchToFiber(fibers[1]); >> + ok(fiberCount == 1, "Wrong fiber count: %d\n", fiberCount); >> + ok(cbCount == 0, "Wrong callback count: %d\n", cbCount); >> + >> + fiberCount = 0; >> + cbCount = 0; >> + fls_value_to_set = val2; >> + pSwitchToFiber(fibers[2]); >> + ok(fiberCount == 1, "Wrong fiber count: %d\n", fiberCount); >> + ok(cbCount == 0, "Wrong callback count: %d\n", cbCount); >> + >> + fls_value_to_set = val2; >> + ret = pFlsSetValue(fls_index_to_set, fls_value_to_set); >> + ok(ret, "FlsSetValue failed\n"); >> + ok(val2 == pFlsGetValue(fls_index_to_set), "FlsGetValue failed\n"); >> + >> + fiberCount = 0; >> + cbCount = 0; >> + fls_value_to_set = val1; >> + pDeleteFiber(fibers[1]); >> + ok(fiberCount == 0, "Wrong fiber count: %d\n", fiberCount); >> + todo_wine >> + { >> + ok(cbCount == 1, "Wrong callback count: %d\n", cbCount); >> + } > > You can make that a one liner: > todo_wine ok(); > > > > > Hope that helps > Have fun!

July 6, 2016

Re: [PATCH v3 6/6] shell32: Set output interfaces to NULL in FolderItems functions.

by Alex Henrie

2016-07-06 1:13 GMT-06:00 Michael Stefaniuc <mstefani(a)redhat.com>: > On 07/06/2016 06:55 AM, Alex Henrie wrote: >> [...] > > This two if's look odd: one for ppid and then for !ppid. > Why don't you just unconditionally do the NULL assignment after the > check below? > >> if (!ppid) >> return E_INVALIDARG; Thanks for catching that. This is a mistake, according to the tests get_Parent should never return E_INVALIDARG. -Alex

July 6, 2016

Ubuntu considering dropping 32-bit libraries

by Josh DuBois

A Slashdot post yesterday pointed to a worrying thread on the Ubuntu mailing list archive about dropping 32-bit support from Ubuntu. The Ubuntu thread is here: https://lists.ubuntu.com/archives/ubuntu-devel/2016-June/039420.html It is not 100% clear to me what they are considering - they mention dropping 32-bit .isos and also ceasing to support users who want to run 32-bit apps (at least unless the app is packaged in a container alongside all of the necessary libraries). Their thought is to do this in a couple of years, with Ubuntu 18.10: 18.10+: * Stop providing i386 port * Run legacy i386 only application in snaps / containers / virtual machines Disturbingly, no one on the Ubuntu thread ever mentions Wine. Oddly there is discussion about IoT applications which might need 32-bit libraries and a proposal to continue building select libs for those things, but maybe minus the graphics stack :-(. In the Slashdot post someone at least mentions Wine, but the little discussion that prompted was seemingly in error until Francois came along to correct them. I think Wine should be aware of the issue and make some noise about it before distros grow attached to the idea of ceasing to build 32-bit libraries. I will probably post to the Ubuntu thread to at least mention Wine, unless someone here is involved in that community and thinks they can do a better job. The more distros hear from the Wine community about the value of 32-bit libraries, the better. It might be that they will conclude on their own that users need them to continue building 32-bit libraries, but I don't know that we can trust distros not to break things if we don't speak up.

July 6, 2016

Re: [PATCH 3/3] wininet: Added semi-stub INTERNET_OPTION_END_BROWSER_SESSION implementation.

by Marvin

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=24024 Your paranoid android. === w7u (32 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14 === w7pro64 (32 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14 === w8 (32 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14 === w864 (32 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14 === w7pro64 (64 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14 === w864 (64 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14

July 6, 2016

Re: [PATCH 2/3] wininet: Always set returned size in InternetGetCookieExA.

by Marvin

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=24023 Your paranoid android. === w7u (32 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14 === w7pro64 (32 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14 === w8 (32 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14 === w864 (32 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14 === w7pro64 (64 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14 === w864 (64 bit internet) === internet.c:366: Test failed: InternetGetCookie should have failed with FALSE and error 14

July 6, 2016

Re: [PATCH 1/6] wined3d: Implement d3d11_immediate_context_Begin().

by Józef Kucia

On Wed, Jul 6, 2016 at 12:02 PM, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > dlls/d3d11/async.c | 5 +++++ > dlls/d3d11/d3d11_private.h | 1 + > dlls/d3d11/device.c | 10 +++++++++- > 3 files changed, 15 insertions(+), 1 deletion(-) The subject prefix should be "d3d11".

July 6, 2016

Re: [PATCH] explorer: add registry support to disable Virtual Desktop systray

by Alexandre Julliard

Rob Walker <bob.mt.wya(a)gmail.com> writes: > Workaround for https://bugs.winehq.org/show_bug.cgi?id=40828 A better approach would be to detect the fullscreen window and automatically hide the taskbar. -- Alexandre Julliard julliard(a)winehq.org

July 6, 2016

Re: [PATCH v3 6/6] shell32: Set output interfaces to NULL in FolderItems functions.

by Michael Stefaniuc

On 07/06/2016 06:55 AM, Alex Henrie wrote: > Cc: Christian Costa <titan.costa(a)gmail.com> > Cc: Sebastian Lackner <sebastian(a)fds-team.de> > > Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> > --- > dlls/shell32/shelldispatch.c | 9 ++++++++- > dlls/shell32/tests/shelldispatch.c | 3 --- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/dlls/shell32/shelldispatch.c b/dlls/shell32/shelldispatch.c > index 6d7973e..2705389 100644 > --- a/dlls/shell32/shelldispatch.c > +++ b/dlls/shell32/shelldispatch.c > @@ -1063,12 +1063,16 @@ static HRESULT WINAPI FolderItemsImpl_get_Application(FolderItems3 *iface, IDisp > if (!ppid) > return E_INVALIDARG; > > + *ppid = NULL; > return E_NOTIMPL; > } > > static HRESULT WINAPI FolderItemsImpl_get_Parent(FolderItems3 *iface, IDispatch **ppid) > { > - FIXME("(%p,%p)\n", iface, ppid); > + TRACE("(%p,%p)\n", iface, ppid); > + > + if (ppid) > + *ppid = NULL; This two if's look odd: one for ppid and then for !ppid. Why don't you just unconditionally do the NULL assignment after the check below? > if (!ppid) > return E_INVALIDARG; bye michael

July 6, 2016

Re: [PATCH v2 1/3] comdlg32: Init clipboard format only once in filedlg.

by Alexandre Julliard

Lauri Kenttä <lauri.kentta(a)gmail.com> writes: > @@ -3798,6 +3789,25 @@ static void COMCTL32_ReleaseStgMedium (STGMEDIUM medium) > } > > /*********************************************************************** > + * GetFormatEtc > + * > + * Get the FORMATETC used in the shell id list > + */ > +static FORMATETC* GetFormatEtc(void) > +{ > + static FORMATETC formatetc; > + if (!formatetc.cfFormat) > + { > + formatetc.cfFormat = RegisterClipboardFormatA(CFSTR_SHELLIDLISTA); > + formatetc.ptd = 0; > + formatetc.dwAspect = DVASPECT_CONTENT; > + formatetc.lindex = -1; > + formatetc.tymed = TYMED_HGLOBAL; > + } > + return &formatetc; > +} This looks a little risky, since the structure is passed to the application. Caching only the clipboard format should be enough. -- Alexandre Julliard julliard(a)winehq.org

July 6, 2016

Re: [PATCH] explorer: add registry support to disable Virtual Desktop systray

by Alex Henrie

+ if (using_root) + { + enable_shell = FALSE; + initialize_systray( graphics_driver, using_root, enable_shell ); + } + else + { + enable_systray=get_default_systray_state(); + initialize_systray( graphics_driver, !enable_systray, enable_shell ); + } I think you should put the same whitespace around the equals sign in both cases. -Alex

July 6, 2016

Re: [1/3] imm32, user32: Create and destroy the default IME window implicitly. (try 8)

by Alexandre Julliard

Akihiro Sagawa <sagawa.aki(a)gmail.com> writes: > Try 8: > - Fix user32 test failures. To achieve this, we create the default IME window > as message-only window for now. This doesn't match Windows behavior, as the tests show: ../../../tools/runtest -q -P wine -T ../../.. -M imm32.dll -p imm32_test.exe.so imm32 && touch imm32.ok imm32.c:209: Test failed: expected IME window existence imm32.c:221: Test failed: expected IME window existence imm32.c:946: Test failed: Expected IME window existence imm32.c:948: Test failed: Expected (nil), got 0x30066 imm32.c:195: Test failed: expected IME window existence imm32.c:209: Test failed: expected IME window existence imm32.c:221: Test failed: expected IME window existence imm32.c:956: Test failed: Expected IME window existence imm32.c:209: Test failed: expected IME window existence imm32.c:221: Test failed: expected IME window existence imm32.c:209: Test failed: expected IME window existence imm32.c:221: Test failed: expected IME window existence imm32.c:994: Test failed: Expected IME window existence imm32.c:996: Test failed: Expected (nil), got 0x50066 imm32.c:209: Test failed: expected IME window existence imm32.c:221: Test failed: expected IME window existence imm32.c:946: Test failed: Expected IME window existence imm32.c:948: Test failed: Expected (nil), got 0x60066 imm32.c:195: Test failed: expected IME window existence imm32.c:209: Test failed: expected IME window existence imm32.c:221: Test failed: expected IME window existence imm32.c:956: Test failed: Expected IME window existence imm32.c:209: Test failed: expected IME window existence imm32.c:221: Test failed: expected IME window existence imm32.c:209: Test failed: expected IME window existence imm32.c:221: Test failed: expected IME window existence imm32.c:994: Test failed: Expected IME window existence imm32.c:996: Test failed: Expected (nil), got 0x80066 imm32.c:946: Test failed: Expected IME window existence imm32.c:948: Test failed: Expected (nil), got 0xb005e imm32.c:195: Test failed: expected IME window existence imm32.c:209: Test failed: expected IME window existence imm32.c:221: Test failed: expected IME window existence imm32.c:956: Test failed: Expected IME window existence imm32.c:946: Test failed: Expected IME window existence imm32.c:948: Test failed: Expected (nil), got 0x20040 imm32.c:195: Test failed: expected IME window existence imm32.c:209: Test failed: expected IME window existence imm32.c:221: Test failed: expected IME window existence imm32.c:956: Test failed: Expected IME window existence make: *** [imm32.ok] Error 40 -- Alexandre Julliard julliard(a)winehq.org

July 6, 2016

Re: [PATCH] advapi32: Add some tests for StartTrace().

by Zebediah Figura

On 07/03/2016 02:38 AM, GOUJON Alexandre wrote: > On 07/02/2016 10:57 PM, Zebediah Figura wrote: >> On 07/02/2016 07:47 AM, Marvin wrote: >>> 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=23920 >>> >>> Your paranoid android. >>> >>> >>> === wxppro (32 bit eventlog) === >>> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1185: Test failed: Expected ERROR_BAD_LENGTH, got 87 >>> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >>> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1272: Test failed: Expected success >>> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >>> >>> === w2003std (32 bit eventlog) === >>> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1185: Test failed: Expected ERROR_BAD_LENGTH, got 87 >>> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >>> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1272: Test failed: Expected success >>> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >>> >>> === wvistau64 (32 bit eventlog) === >>> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >>> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1272: Test failed: Expected success >>> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >>> >>> === w2008s64 (32 bit eventlog) === >>> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >>> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1272: Test failed: Expected success >>> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >>> >>> === w7u (32 bit eventlog) === >>> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >>> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1272: Test failed: Expected success >>> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >>> >>> === w7pro64 (32 bit eventlog) === >>> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >>> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1272: Test failed: Expected success >>> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >>> >>> === w8 (32 bit eventlog) === >>> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >>> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1272: Test failed: Expected success >>> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >>> >>> === w864 (32 bit eventlog) === >>> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >>> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1272: Test failed: Expected success >>> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >>> >>> === w1064 (32 bit eventlog) === >>> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >>> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >>> eventlog.c:1272: Test failed: Expected success >>> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >>> >>> >> This test is failing when built 32 bits, but I tried building both >> 64-bit and 32-bit executables using mingw-w64 on my win7 machine, and >> they both worked. I'm a newbie (this is my first patch), so I'm probably >> doing something wrong, but I don't know what. The error in question (87) >> is probably associated with the wrong sizeof() being returned, but I >> don't know why that would happen. >> >> Thanks, >> Zeb > Welcome to wine ! > > Nothing really wrong at a first look but we can nitpick (inconsistent > formatting, PFOO usage instead of FOO *, etc.). > Returned error may depend on Windows version, installed Service Pack, > dll version, structure version... > To broaden your tests, you can use the test bot > (https://wiki.winehq.org/Wine_TestBot) > > In your case, it may be an alignment issue (see > http://stackoverflow.com/questions/9049971/windows-etw-starttrace-failing-w…) > > I tried fixing the alignment issue by padding, but this raised ERROR_BAD_PATHNAME instead. I can't figure out why. I checked using gdb, and LogFileNameOffset points to a valid pathname, and the other apparent condition (LogFileMode = 0) is not met. I can't figure out why this error is being raised.

July 5, 2016

Re: [PATCH 3/3] regedit: Process the next file, if any, on error (v2)

by Hugh McMaster

On Wednesday, 6 July 2016 12:09 AM, Sebastian Lackner wrote: >On 05.07.2016 11:13, Hugh McMaster wrote: >> @@ -113,7 +113,9 @@ static BOOL PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i) >> if (size == 0) >> { >> output_message(STRING_FILE_NOT_FOUND, filename); >> - exit(1); >> + if (realname) >> + HeapFree(GetProcessHeap(), 0, realname); > > I haven't reviewed the whole series, but passing a NULL pointer to HeapFree() > is perfectly fine, and its better to avoid such checks. Okay, thanks for pointing that out. Alexandre has already fixed this in both places (see http://source.winehq.org/git/wine.git/commitdiff/6ad44d7a3190d8e9a3daa9bd72…) so no action is necessary. Thanks for the review.

July 5, 2016

[v3] secur32 : add and fix some algids

by Max Qian

On 2016-07-05 5:55 am, Jacek Caban wrote: > Hi Max, > > On 03.07.2016 23:41, Max Qian wrote: >> diff --git a/include/wincrypt.h b/include/wincrypt.h >> index 55c9509..19af6ca 100644 >> --- a/include/wincrypt.h >> +++ b/include/wincrypt.h >> @@ -1583,6 +1583,7 @@ typedef const CERT_CRL_CONTEXT_PAIR >> *PCCERT_CRL_CONTEXT_PAIR; >> #define ALG_SID_AGREED_KEY_ANY 3 >> #define ALG_SID_KEA 4 >> #define ALG_SID_ECDH 5 >> +#define ALG_SID_ECDH_EPHEM 6 >> /* RC2 SIDs */ >> #define ALG_SID_RC4 1 >> #define ALG_SID_RC2 2 >> @@ -1639,6 +1640,7 @@ typedef const CERT_CRL_CONTEXT_PAIR >> *PCCERT_CRL_CONTEXT_PAIR; >> #define CALG_KEA_KEYX (ALG_CLASS_KEY_EXCHANGE | >> ALG_TYPE_DH | ALG_SID_KEA) >> #define CALG_HUGHES_MD5 (ALG_CLASS_KEY_EXCHANGE | >> ALG_TYPE_ANY | ALG_SID_MD5) >> #define CALG_ECDH (ALG_CLASS_KEY_EXCHANGE | >> ALG_TYPE_DH | ALG_SID_ECDH) >> +#define CALG_ECDH_EPHEM (ALG_CLASS_KEY_EXCHANGE | >> ALG_TYPE_DH | ALG_SID_ECDH_EPHEM) >> #define CALG_RSA_KEYX (ALG_CLASS_KEY_EXCHANGE | >> ALG_TYPE_RSA | ALG_SID_RSA_ANY) >> #define CALG_ECMQV (ALG_CLASS_KEY_EXCHANGE | >> ALG_TYPE_ANY | ALG_SID_ECMQV) >> #define CALG_DES (ALG_CLASS_DATA_ENCRYPT | >> ALG_TYPE_BLOCK | ALG_SID_DES) > > Do you have a version of PSDK that has this definition? I can't see it > here. If those are not in MS headers, we shouldn't have them unless we > have a strong reason. When I asked you about adding a comment, I was > hoping for something like > > /* MSDN mentions CALG_ECDH_EPHEM, but it's not available in headers. */ > case ....: return CALG_ECDH; > > Thanks, > Jacek For v3 I replaced CALG_ECDH_EPHEM with a comment in the header file, because both backends have locations where CALG_ECDH_EPHEM could be used. I thought you meant that CALG_ECDH_EPHEM wasn't in the Wine header files, whoops.

July 5, 2016

Re: [PATCH 02/15] mshtml: Rewrite navigation tests using new HTTP emulation framework.

by Jacek Caban

On 7/5/16 10:51 PM, Marvin wrote: > 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=24000 > > Your paranoid android. > > > === build (build) === > Make failed It looks like testbot is missing make depend step. Jacek

July 5, 2016

Re: [PATCH 14/15] mshtml: Reimplement IHTMLDocument5::get_compatMode based on document mode.

by Marvin

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=24002 Your paranoid android. === build (build) === Make failed

July 5, 2016

Re: [PATCH 13/15] mshtml: Added document.documentMode tests.

by Marvin

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=24001 Your paranoid android. === build (build) === Make failed

July 5, 2016

Re: [PATCH 02/15] mshtml: Rewrite navigation tests using new HTTP emulation framework.

by Marvin

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=24000 Your paranoid android. === build (build) === Make failed

July 5, 2016

Re: [PATCH] ws2_32: Allow user to enable IP dual stack (v3)

by Bruno Jesus

On Tue, Jul 5, 2016 at 10:33 AM, Matthieu Nottale <matthieu.nottale(a)infinit.io> wrote: > IP dual stack (v4+v6) should be disabled by default, but previous code > was setting IPV6_V6ONLY in bind() which prevented user to override it. > This patch moves setting IPV6_V6ONLY to socket creation time. It still looks like there is a formatting problem with the patch, please attach it in a new attempt or use git send-email, also see the comments below. > Signed-off-by: Matthieu Nottale <matthieu.nottale(a)infinit.io> > --- > dlls/ws2_32/socket.c | 22 ++++++++------------- > dlls/ws2_32/tests/sock.c | 51 > ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 59 insertions(+), 14 deletions(-) > > + enabled = 2; > + len = sizeof(enabled); > + ret = getsockopt(v6, IPPROTO_IPV6, IPV6_V6ONLY, &enabled, &len); > + if (ret) { > + ok(!ret, "Failed to check IPV6_V6ONLY ((LastError: %d).\n", > WSAGetLastError()); > + goto end; > + } If the ok() is supposed to always fail use ok(0, ...). But according to msdn this option is not present in 2003/XP, so you would end up with an error. Better to use win_skip then to avoid this problem. The testbot will test against many different Windows versions so we need to ensure it works everywhere, before resending it would be useful to do a manual run at https://testbot.winehq.org/ to check the patch. > + ok(enabled == 1, "IPV6_V6ONLY is not enabled by default.\n"); > + ret = setsockopt(v6, IPPROTO_IPV6, IPV6_V6ONLY, &enabled, len); > + if (ret) { > + ok(!ret, "Failed to disable IPV6_V6ONLY (LastError: %d).\n", > WSAGetLastError()); > + goto end; > + } This should never fail as the previous test already took care of checking for IPV6_V6ONLY support, so drop the if and goto. > + enabled = 2; > + len = sizeof(enabled); > + ret = getsockopt(v6, IPPROTO_IPV6, IPV6_V6ONLY, &enabled, &len); > + if (ret) { > + ok(!ret, "Failed to check IPV6_V6ONLY (LastError: %d).\n", > WSAGetLastError()); > + goto end; > + } Again I think getsockopt must always work at this point. > + ok(!enabled, "IPV6_V6ONLY is enabled after bind.\n"); > + v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); > + if (v4 == INVALID_SOCKET) { > + skip("Could not create IPv4 socket (LastError: %d).\n", > + WSAGetLastError()); > + goto end; > + } > + WSASetLastError(0xdeadbeef); > + ret = bind(v4, (struct sockaddr*)&sin4, sizeof(sin4)); Please also test ok(ret,...); Best wishes, Bruno

July 5, 2016

Re: [PATCH v2 3/5] shell32/tests: Add tests for FolderItems.

by Nikolay Sivov

On 05.07.2016 20:04, Alex Henrie wrote: > 2016-07-05 10:59 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: >> On 05.07.2016 19:56, Alex Henrie wrote: >>> 2016-07-05 10:54 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: >>>> On 05.07.2016 18:46, Alex Henrie wrote: >>>>> 2016-07-05 0:26 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: >>>>>> On 05.07.2016 8:47, Alex Henrie wrote: >>>>>>> + if (0) /* crashes on all versions of Windows */ >>>>>>> + { >>>>>>> + r = FolderItems_get_Count(items, NULL); >>>>>>> +todo_wine >>>>>>> + ok(r == E_INVALIDARG, "expected E_INVALIDARG, got %08x\n", r); >>>>>>> + } >>>>>> >>>>>> Why do you want to handle this case if it crashes consistently on >>>>>> Windows? And todo_wine is not going to do anything in commented block >>>>>> obviously. >>>>> >>>>> Good point. Would you like me to remove this code altogether, or just >>>>> the todo_wine and ok statement? Also, would you like me to change the >>>>> other if (0) blocks to if (GetVersion() > MAKELONG(5,1)) to avoid >>>>> having todo_wine's in if (0) blocks? >>>> >>>> If it crashes on some windows versions just put it in 'if (0)' with >>>> appropriate comment for documentation purposes, and a NULL check could >>>> be implemented. Version 5,1 means XP, right? if it does not crash since >>>> XP just make it a regular test, enabled unconditionally. >>> >>> It crashes on XP, and I'm pretty sure that it's not OK to have tests >>> that crash the XP testbot. I'll remove the todo_wine's though. >> >> Ah, right, it was a greater-than check. So yes, if it only crashes on >> XP, and works on everything newer we should implement newer behavior and >> have disabled test that documents all that. No need for todo_wine. > > And you don't want any if (0) blocks that crash on all versions of > Windows, right? I usually add them, but I don't think it's a strict requirement. > > -Alex >

July 5, 2016

Re: [PATCH v2 3/5] shell32/tests: Add tests for FolderItems.

by Alex Henrie

2016-07-05 10:59 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: > On 05.07.2016 19:56, Alex Henrie wrote: >> 2016-07-05 10:54 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: >>> On 05.07.2016 18:46, Alex Henrie wrote: >>>> 2016-07-05 0:26 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: >>>>> On 05.07.2016 8:47, Alex Henrie wrote: >>>>>> + if (0) /* crashes on all versions of Windows */ >>>>>> + { >>>>>> + r = FolderItems_get_Count(items, NULL); >>>>>> +todo_wine >>>>>> + ok(r == E_INVALIDARG, "expected E_INVALIDARG, got %08x\n", r); >>>>>> + } >>>>> >>>>> Why do you want to handle this case if it crashes consistently on >>>>> Windows? And todo_wine is not going to do anything in commented block >>>>> obviously. >>>> >>>> Good point. Would you like me to remove this code altogether, or just >>>> the todo_wine and ok statement? Also, would you like me to change the >>>> other if (0) blocks to if (GetVersion() > MAKELONG(5,1)) to avoid >>>> having todo_wine's in if (0) blocks? >>> >>> If it crashes on some windows versions just put it in 'if (0)' with >>> appropriate comment for documentation purposes, and a NULL check could >>> be implemented. Version 5,1 means XP, right? if it does not crash since >>> XP just make it a regular test, enabled unconditionally. >> >> It crashes on XP, and I'm pretty sure that it's not OK to have tests >> that crash the XP testbot. I'll remove the todo_wine's though. > > Ah, right, it was a greater-than check. So yes, if it only crashes on > XP, and works on everything newer we should implement newer behavior and > have disabled test that documents all that. No need for todo_wine. And you don't want any if (0) blocks that crash on all versions of Windows, right? -Alex

July 5, 2016

Re: [PATCH v2 3/5] shell32/tests: Add tests for FolderItems.

by Nikolay Sivov

On 05.07.2016 19:56, Alex Henrie wrote: > 2016-07-05 10:54 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: >> On 05.07.2016 18:46, Alex Henrie wrote: >>> 2016-07-05 0:26 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: >>>> On 05.07.2016 8:47, Alex Henrie wrote: >>>>> + if (0) /* crashes on all versions of Windows */ >>>>> + { >>>>> + r = FolderItems_get_Count(items, NULL); >>>>> +todo_wine >>>>> + ok(r == E_INVALIDARG, "expected E_INVALIDARG, got %08x\n", r); >>>>> + } >>>> >>>> Why do you want to handle this case if it crashes consistently on >>>> Windows? And todo_wine is not going to do anything in commented block >>>> obviously. >>> >>> Good point. Would you like me to remove this code altogether, or just >>> the todo_wine and ok statement? Also, would you like me to change the >>> other if (0) blocks to if (GetVersion() > MAKELONG(5,1)) to avoid >>> having todo_wine's in if (0) blocks? >> >> If it crashes on some windows versions just put it in 'if (0)' with >> appropriate comment for documentation purposes, and a NULL check could >> be implemented. Version 5,1 means XP, right? if it does not crash since >> XP just make it a regular test, enabled unconditionally. > > It crashes on XP, and I'm pretty sure that it's not OK to have tests > that crash the XP testbot. I'll remove the todo_wine's though. Ah, right, it was a greater-than check. So yes, if it only crashes on XP, and works on everything newer we should implement newer behavior and have disabled test that documents all that. No need for todo_wine. > > -Alex >

July 5, 2016

Re: [PATCH v2 3/5] shell32/tests: Add tests for FolderItems.

by Alex Henrie

2016-07-05 10:54 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: > On 05.07.2016 18:46, Alex Henrie wrote: >> 2016-07-05 0:26 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: >>> On 05.07.2016 8:47, Alex Henrie wrote: >>>> + if (0) /* crashes on all versions of Windows */ >>>> + { >>>> + r = FolderItems_get_Count(items, NULL); >>>> +todo_wine >>>> + ok(r == E_INVALIDARG, "expected E_INVALIDARG, got %08x\n", r); >>>> + } >>> >>> Why do you want to handle this case if it crashes consistently on >>> Windows? And todo_wine is not going to do anything in commented block >>> obviously. >> >> Good point. Would you like me to remove this code altogether, or just >> the todo_wine and ok statement? Also, would you like me to change the >> other if (0) blocks to if (GetVersion() > MAKELONG(5,1)) to avoid >> having todo_wine's in if (0) blocks? > > If it crashes on some windows versions just put it in 'if (0)' with > appropriate comment for documentation purposes, and a NULL check could > be implemented. Version 5,1 means XP, right? if it does not crash since > XP just make it a regular test, enabled unconditionally. It crashes on XP, and I'm pretty sure that it's not OK to have tests that crash the XP testbot. I'll remove the todo_wine's though. -Alex

July 5, 2016

Re: [PATCH v2 3/5] shell32/tests: Add tests for FolderItems.

by Nikolay Sivov

On 05.07.2016 18:46, Alex Henrie wrote: > 2016-07-05 0:26 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: >> On 05.07.2016 8:47, Alex Henrie wrote: >>> + if (0) /* crashes on all versions of Windows */ >>> + { >>> + r = FolderItems_get_Count(items, NULL); >>> +todo_wine >>> + ok(r == E_INVALIDARG, "expected E_INVALIDARG, got %08x\n", r); >>> + } >> >> Why do you want to handle this case if it crashes consistently on >> Windows? And todo_wine is not going to do anything in commented block >> obviously. > > Good point. Would you like me to remove this code altogether, or just > the todo_wine and ok statement? Also, would you like me to change the > other if (0) blocks to if (GetVersion() > MAKELONG(5,1)) to avoid > having todo_wine's in if (0) blocks? If it crashes on some windows versions just put it in 'if (0)' with appropriate comment for documentation purposes, and a NULL check could be implemented. Version 5,1 means XP, right? if it does not crash since XP just make it a regular test, enabled unconditionally. > > -Alex >

July 5, 2016

Re: [PATCH v2 3/5] shell32/tests: Add tests for FolderItems.

by Alex Henrie

2016-07-05 0:26 GMT-06:00 Nikolay Sivov <bunglehead(a)gmail.com>: > On 05.07.2016 8:47, Alex Henrie wrote: >> + if (0) /* crashes on all versions of Windows */ >> + { >> + r = FolderItems_get_Count(items, NULL); >> +todo_wine >> + ok(r == E_INVALIDARG, "expected E_INVALIDARG, got %08x\n", r); >> + } > > Why do you want to handle this case if it crashes consistently on > Windows? And todo_wine is not going to do anything in commented block > obviously. Good point. Would you like me to remove this code altogether, or just the todo_wine and ok statement? Also, would you like me to change the other if (0) blocks to if (GetVersion() > MAKELONG(5,1)) to avoid having todo_wine's in if (0) blocks? -Alex

July 5, 2016

Re: [PATCH 6/7] d3drm: Implement IDirect3DRMViewport*::Init. (v3)

by Henri Verbeet

On 27 June 2016 at 23:54, Aaryaman Vasishta <jem456.vasishta(a)gmail.com> wrote: > static HRESULT WINAPI d3drm_viewport1_Init(IDirect3DRMViewport *iface, IDirect3DRMDevice *device, > IDirect3DRMFrame *camera, DWORD x, DWORD y, DWORD width, DWORD height) > { > - FIXME("iface %p, device %p, camera %p, x %u, y %u, width %u, height %u stub!\n", > - iface, device, camera, x, y, width, height); > + struct d3drm_viewport *viewport = impl_from_IDirect3DRMViewport(iface); > + struct d3drm_frame *frame = impl_from_IDirect3DRMFrame(camera); Using impl_from_IDirect3DRMFrame() here is unusual. > + if (!device || !camera) > + return D3DRMERR_BADOBJECT; And note that this only works because of the d3drm_frame structure layout. You'll want to use unsafe_impl_from_IDirect3DRMFrame() if "camera" can be NULL.

July 5, 2016

Re: ntdll: Add RtlRestoreContext implementation

by Piotr Caban

On 07/05/16 16:07, Sebastian Lackner wrote: > On 04.07.2016 10:55, Piotr Caban wrote: >> Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> >> --- >> dlls/kernel32/kernel32.spec | 2 +- >> dlls/ntdll/ntdll.spec | 1 + >> dlls/ntdll/signal_x86_64.c | 73 +++++++++++++++++++++++++-------------------- >> 3 files changed, 42 insertions(+), 34 deletions(-) >> > > Your patch also changes unwinding behavior in case of MSVCRT_JUMP_BUFFER. > Previously the instruction pointer stored in the jump buffer was ignored, > now it will overwrite the value passed as target_ip. Is that intentional? My bad, I'll send a fixed version. Thank you, Piotr

July 5, 2016

Re: [PATCH 3/3] regedit: Process the next file, if any, on error (v2)

by Sebastian Lackner

On 05.07.2016 11:13, Hugh McMaster wrote: > Changes since v1: > * Do not leak memory when moving to the next file. > > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > programs/regedit/regedit.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/programs/regedit/regedit.c b/programs/regedit/regedit.c > index 04dd317..8a6a975 100644 > --- a/programs/regedit/regedit.c > +++ b/programs/regedit/regedit.c > @@ -88,7 +88,7 @@ typedef enum { > ACTION_ADD, ACTION_EXPORT, ACTION_DELETE > } REGEDIT_ACTION; > > -static BOOL PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i) > +static void PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i) > { > switch (action) { > case ACTION_ADD: { > @@ -113,7 +113,9 @@ static BOOL PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i) > if (size == 0) > { > output_message(STRING_FILE_NOT_FOUND, filename); > - exit(1); > + if (realname) > + HeapFree(GetProcessHeap(), 0, realname); I haven't reviewed the whole series, but passing a NULL pointer to HeapFree() is perfectly fine, and its better to avoid such checks. > + return; > } > reg_file = _wfopen(realname, rb_mode); > if (reg_file == NULL) > @@ -121,7 +123,9 @@ static BOOL PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i) > WCHAR regedit[] = {'r','e','g','e','d','i','t',0}; > _wperror(regedit); > output_message(STRING_CANNOT_OPEN_FILE, filename); > - exit(1); > + if (realname) > + HeapFree(GetProcessHeap(), 0, realname); Same here. > + return; > } > import_registry_file(reg_file); > if (realname) > @@ -150,7 +154,6 @@ static BOOL PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i) > exit(1); > break; > } > - return TRUE; > } > > BOOL ProcessCmdLine(WCHAR *cmdline) >

July 5, 2016

Re: ntdll: Add RtlRestoreContext implementation

by Sebastian Lackner

On 04.07.2016 10:55, Piotr Caban wrote: > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/kernel32/kernel32.spec | 2 +- > dlls/ntdll/ntdll.spec | 1 + > dlls/ntdll/signal_x86_64.c | 73 +++++++++++++++++++++++++-------------------- > 3 files changed, 42 insertions(+), 34 deletions(-) > Your patch also changes unwinding behavior in case of MSVCRT_JUMP_BUFFER. Previously the instruction pointer stored in the jump buffer was ignored, now it will overwrite the value passed as target_ip. Is that intentional?

July 5, 2016

Re: [PATCH] ws2_32: Allow user to enable IP dual stack (v3)

by Marvin

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=23992 Your paranoid android. === build (build) === Patch failed to apply

July 5, 2016

Re: [v2] secur32 : add and fix some algids

by Jacek Caban

Hi Max, On 03.07.2016 23:41, Max Qian wrote: > diff --git a/include/wincrypt.h b/include/wincrypt.h > index 55c9509..19af6ca 100644 > --- a/include/wincrypt.h > +++ b/include/wincrypt.h > @@ -1583,6 +1583,7 @@ typedef const CERT_CRL_CONTEXT_PAIR *PCCERT_CRL_CONTEXT_PAIR; > #define ALG_SID_AGREED_KEY_ANY 3 > #define ALG_SID_KEA 4 > #define ALG_SID_ECDH 5 > +#define ALG_SID_ECDH_EPHEM 6 > /* RC2 SIDs */ > #define ALG_SID_RC4 1 > #define ALG_SID_RC2 2 > @@ -1639,6 +1640,7 @@ typedef const CERT_CRL_CONTEXT_PAIR *PCCERT_CRL_CONTEXT_PAIR; > #define CALG_KEA_KEYX (ALG_CLASS_KEY_EXCHANGE | ALG_TYPE_DH | ALG_SID_KEA) > #define CALG_HUGHES_MD5 (ALG_CLASS_KEY_EXCHANGE | ALG_TYPE_ANY | ALG_SID_MD5) > #define CALG_ECDH (ALG_CLASS_KEY_EXCHANGE | ALG_TYPE_DH | ALG_SID_ECDH) > +#define CALG_ECDH_EPHEM (ALG_CLASS_KEY_EXCHANGE | ALG_TYPE_DH | ALG_SID_ECDH_EPHEM) > #define CALG_RSA_KEYX (ALG_CLASS_KEY_EXCHANGE | ALG_TYPE_RSA | ALG_SID_RSA_ANY) > #define CALG_ECMQV (ALG_CLASS_KEY_EXCHANGE | ALG_TYPE_ANY | ALG_SID_ECMQV) > #define CALG_DES (ALG_CLASS_DATA_ENCRYPT | ALG_TYPE_BLOCK | ALG_SID_DES) Do you have a version of PSDK that has this definition? I can't see it here. If those are not in MS headers, we shouldn't have them unless we have a strong reason. When I asked you about adding a comment, I was hoping for something like /* MSDN mentions CALG_ECDH_EPHEM, but it's not available in headers. */ case ....: return CALG_ECDH; Thanks, Jacek

July 5, 2016

Re: krnl386.exe16: Fix a typo in comment.

by Huw Davies

On Tue, Jul 05, 2016 at 03:36:30PM +0300, Andrey Gusev wrote: > diff --git a/dlls/krnl386.exe16/file.c b/dlls/krnl386.exe16/file.c > index 4e7f7c4..44b1e75 100644 > --- a/dlls/krnl386.exe16/file.c > +++ b/dlls/krnl386.exe16/file.c > @@ -549,7 +549,7 @@ INT16 WINAPI GetPrivateProfileString16( LPCSTR section, LPCSTR entry, > if (!entry) > { > /* We have to return the list of keys in the section but without the values > - * so we need to massage the results of GetPrivateProfileSectionA. > + * so we need to message the results of GetPrivateProfileSectionA. > */ > UINT ret, oldlen = len, size = min( len, 1024 ); > LPSTR data, src; No, again the original is correct. Huw.

July 5, 2016

Re: gdi32/dibdrv: Fix a typo in comment.

by Huw Davies

On Tue, Jul 05, 2016 at 03:33:39PM +0300, Andrey Gusev wrote: > diff --git a/dlls/gdi32/dibdrv/graphics.c b/dlls/gdi32/dibdrv/graphics.c > index bfb3451..ca357b2 100644 > --- a/dlls/gdi32/dibdrv/graphics.c > +++ b/dlls/gdi32/dibdrv/graphics.c > @@ -1567,7 +1567,7 @@ COLORREF dibdrv_SetPixel( PHYSDEV dev, INT x, INT y, COLORREF color ) > rect.bottom = rect.top + 1; > add_clipped_bounds( pdev, &rect, pdev->clip ); > > - /* SetPixel doesn't do the 1bpp massaging like other fg colors */ > + /* SetPixel doesn't do the 1bpp messaging like other fg colors */ > pixel = get_pixel_color( dev->hdc, &pdev->dib, color, FALSE ); > color = pdev->dib.funcs->pixel_to_colorref( &pdev->dib, pixel ); No, the original is correct. Huw.

July 5, 2016

Re: [PATCH 2/5] shell32: Stub out FolderItems.

by Michael Stefaniuc

Alex, On 07/05/2016 07:50 AM, Alex Henrie wrote: > I grepped the code and the names ppTInfo, rgszNames, and rgDispId are > used pretty consistently throughout Wine. I think that unless we're > going to refactor the names across the entire codebase, we might as > well keep them. you miss the point. That is *old* code when Alexandre was "still young and needed the money"... The rules have changed and new code should be clean even if it is surrounded by old ugly code. bye michael

July 5, 2016

Re: [PATCH v2 3/5] shell32/tests: Add tests for FolderItems.

by Nikolay Sivov

On 05.07.2016 8:47, Alex Henrie wrote: > + if (0) /* crashes on all versions of Windows */ > + { > + r = FolderItems_get_Count(items, NULL); > +todo_wine > + ok(r == E_INVALIDARG, "expected E_INVALIDARG, got %08x\n", r); > + } Why do you want to handle this case if it crashes consistently on Windows? And todo_wine is not going to do anything in commented block obviously.

July 5, 2016

Re: [PATCH v2 2/5] shell32: Stub out FolderItems.

by Nikolay Sivov

On 05.07.2016 8:47, Alex Henrie wrote: > Cc: Christian Costa <titan.costa(a)gmail.com> > Cc: Sebastian Lackner <sebastian(a)fds-team.de> > > Wine Staging has included a similar patch since 1.7.51, however this > patch is not based on theirs. > > FolderItems2 worked in Windows 2000, but it was removed when > FolderItems3 was introduced in Windows XP. However, it doesn't hurt us > to provide this interface anyway for any old applications that might > want it. > > Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> > --- > dlls/shell32/shell32_main.h | 1 + > dlls/shell32/shelldispatch.c | 207 ++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 206 insertions(+), 2 deletions(-) > > diff --git a/dlls/shell32/shell32_main.h b/dlls/shell32/shell32_main.h > index 492f79f..64a26bb 100644 > --- a/dlls/shell32/shell32_main.h > +++ b/dlls/shell32/shell32_main.h > @@ -234,6 +234,7 @@ enum tid_t { > IShellDispatch6_tid, > IShellFolderViewDual3_tid, > Folder3_tid, > + FolderItems3_tid, > FolderItem2_tid, > FolderItemVerb_tid, > FolderItemVerbs_tid, That's not going to work, you have to update tid_ids[] too. Also please keep it sorted. > diff --git a/dlls/shell32/shelldispatch.c b/dlls/shell32/shelldispatch.c > index 6dece6d..3661430 100644 > --- a/dlls/shell32/shelldispatch.c > +++ b/dlls/shell32/shelldispatch.c > @@ -65,6 +65,11 @@ typedef struct { > } FolderImpl; > > typedef struct { > + FolderItems3 FolderItems3_iface; > + LONG ref; > +} FolderItemsImpl; > + > +typedef struct { > FolderItem2 FolderItem2_iface; > LONG ref; > VARIANT dir; > @@ -97,6 +102,11 @@ static inline FolderImpl *impl_from_Folder(Folder3 *iface) > return CONTAINING_RECORD(iface, FolderImpl, Folder3_iface); > } > > +static inline FolderItemsImpl *impl_from_FolderItems(FolderItems3 *iface) > +{ > + return CONTAINING_RECORD(iface, FolderItemsImpl, FolderItems3_iface); > +} > + > static inline FolderItemImpl *impl_from_FolderItem(FolderItem2 *iface) > { > return CONTAINING_RECORD(iface, FolderItemImpl, FolderItem2_iface); > @@ -939,6 +949,200 @@ static HRESULT FolderItem_Constructor(VARIANT *dir, FolderItem **ppfi) > return ret; > } > > +static HRESULT WINAPI FolderItemsImpl_QueryInterface(FolderItems3 *iface, > + REFIID riid, LPVOID *ppv) > +{ > + FolderItemsImpl *This = impl_from_FolderItems(iface); > + > + TRACE("(%p,%p,%p)\n", iface, riid, ppv); Usually we trace riid. > + > + if (!ppv) return E_INVALIDARG; This is unusual, do we have a test for that?

July 5, 2016

Re: [PATCH 2/5] shell32: Stub out FolderItems.

by Alex Henrie

2016-07-04 8:37 GMT-06:00 Alex Henrie <alexhenrie24(a)gmail.com>: > 2016-07-04 5:09 GMT-06:00 Sebastian Lackner <sebastian(a)fds-team.de>: >> On 04.07.2016 07:25, Alex Henrie wrote: >>> [...] >> >> Some of the variable names (for example ppTInfo, rgszNames, rgDispId) sound >> a bit odd. Usually variable names without ugly prefix are preferred for new >> code (except we want to keep it for compatibility with old code here?). > > I was trying to make the new code match the surrounding code. I don't > think the variable names matter very much here, but I'll change them > if you feel strongly about it. > >>> +static HRESULT WINAPI FolderItemsImpl_Item(FolderItems3 *iface, VARIANT index, FolderItem **ppid) >>> +{ >>> + FIXME("(%p,%p)\n", iface, ppid); >> >> You forgot to trace the index parameter here. > > Good point. I'll fix that and resubmit the series (although hopefully > Alexandre will have taken patch 1 without changes). > >>> @@ -1093,8 +1297,7 @@ static HRESULT WINAPI FolderImpl_Items(Folder3 *iface, FolderItems **ppid) >>> { >>> FIXME("(%p,%p)\n", iface, ppid); >> >> You forgot to change the FIXME to a TRACE. > > This is actually intentional, because the patch makes this function > return a list of items, but the list is empty. I'm going to change the > FIXME to a TRACE in the patch that populates the list: > https://github.com/alexhenrie/wine/commits/master > > Thanks for the feedback. > > -Alex I grepped the code and the names ppTInfo, rgszNames, and rgDispId are used pretty consistently throughout Wine. I think that unless we're going to refactor the names across the entire codebase, we might as well keep them. So, the only change I've made between v1 and v2 of this patchset is adding the missing parameter to FolderItemsImpl_Item's FIXME statement. -Alex

July 5, 2016

Re: PATCH] ws2_32: Allow user to enable IP dual stack.

by Bruno Jesus

On Mon, Jul 4, 2016 at 6:35 AM, Matthieu Nottale <matthieu.nottale(a)infinit.io> wrote: > IP dual stack (v4+v6) should be disabled by default, but previous code > was setting IPV6_V6ONLY in bind() which prevented user to override it. > This patch moves setting IPV6_V6ONLY to socket creation time. Hi, thanks for the updated version. I believe the patch failed to apply because you added text after it and also an attachment which is inlined while the email is being parsed. The result you can see at [1]. The subject should show how many attempts the patch was sent to, so next subject should be: [PATCH] ws2_32: Allow user to enable IP dual stack (v3) Please see some nitpicks below. [1] http://source.winehq.org/patches/data/123921 > Signed-off-by: Matthieu Nottale <matthieu.nottale(a)infinit.io> > --- > dlls/ws2_32/socket.c | 22 ++++++++-------------- > dlls/ws2_32/tests/sock.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 48 insertions(+), 14 deletions(-) > > diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c > index b0af3d7..c29bd2b 100644 > --- a/dlls/ws2_32/socket.c > +++ b/dlls/ws2_32/socket.c > @@ -3240,20 +3240,6 @@ int WINAPI WS_bind(SOCKET s, const struct > WS_sockaddr* name, int namelen) > } > else > { > -#ifdef IPV6_V6ONLY > - const struct sockaddr_in6 *in6 = (const struct > sockaddr_in6*) &uaddr; > - if (name->sa_family == WS_AF_INET6 && > - !memcmp(&in6->sin6_addr, &in6addr_any, sizeof(struct > in6_addr))) > - { > - int enable = 1; > - if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &enable, > sizeof(enable)) == -1) > - { > - release_sock_fd( s, fd ); > - SetLastError(WSAEAFNOSUPPORT); > - return SOCKET_ERROR; > - } > - } > -#endif > if (name->sa_family == WS_AF_INET) > { > struct sockaddr_in *in4 = (struct sockaddr_in*) &uaddr; > @@ -7163,6 +7149,14 @@ SOCKET WINAPI WSASocketW(int af, int type, int > protocol, > TRACE("\tcreated %04lx\n", ret ); > if (ipxptype > 0) > set_ipx_packettype(ret, ipxptype); > +#ifdef IPV6_V6ONLY > + if (unixaf == AF_INET6) > + { > + /* Winsock documentation stipulates that IPV6_V6ONLY is enabled > by default*/ The documentation is not always useful (and sometimes wrong) and now we should have a test to back this up, I believe a better comment to be: /* AF_INET6 sockets have IPV6_V6ONLY enabled by default. */ Watch out for missing space between the last comment word and the */ > + int enable = 1; > + WS_setsockopt(ret, WS_IPPROTO_IPV6, WS_IPV6_V6ONLY, &enable, > sizeof(enable)); > + } > +#endif > return ret; > } > > diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c > index 6853279..7c8d834 100644 > --- a/dlls/ws2_32/tests/sock.c > +++ b/dlls/ws2_32/tests/sock.c > @@ -6114,6 +6114,46 @@ static void test_ipv6only(void) > ok(!ret, "Could not bind IPv4 address (LastError: %d; %d expected if > IPv6 binds to IPv4 as well).\n", > WSAGetLastError(), WSAEADDRINUSE); > > + if (v4 != INVALID_SOCKET) > + closesocket(v4); > + if (v6 != INVALID_SOCKET) > + closesocket(v6); > + > + /* Test again, this time disabling V6ONLY*/ > + v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP); > + if (v6 == INVALID_SOCKET) { > + skip("Could not create IPv6 socket (LastError: %d; %d expected if > IPv6 not available).\n", > + WSAGetLastError(), WSAEAFNOSUPPORT); > + goto end; > + } Do a getsockopt at this point to check what is the default enabled value. Set enabled to the opposite of what you expected before the call and test its value after. > + DWORD enabled = 0; > + int len = sizeof(enabled); You cannot declare variables like this in C89 (?) so do it in the function beginning. > + ret = setsockopt(v6, IPPROTO_IPV6, IPV6_V6ONLY, &enabled, len); > + if (ret) { > + ok(!ret, "Failed to disable IPV6_V6ONLY (LastError: %d).\n", > WSAGetLastError()); > + goto end; > + } I understand that you are trying to skip when IPV6_V6ONLY is not present. It will be better if you catch the explicit error case and bail out, something like: WSASetLastError(0xdeadbeef); ret = setsockopt(v6, IPPROTO_IPV6, IPV6_V6ONLY, &enabled, len); error = WSAGetLastError(); if (ret) { ok(error == EXPECTED_ERROR, "Failed to disable IPV6_V6ONLY (LastError %d, %d expected).\n", error, EXPECTED_ERROR); goto end; } > ... Best wishes, Bruno

July 4, 2016

Re: [PATCH 2/5] shell32: Stub out FolderItems.

by Alex Henrie

2016-07-04 5:09 GMT-06:00 Sebastian Lackner <sebastian(a)fds-team.de>: > On 04.07.2016 07:25, Alex Henrie wrote: >> [...] > > Some of the variable names (for example ppTInfo, rgszNames, rgDispId) sound > a bit odd. Usually variable names without ugly prefix are preferred for new > code (except we want to keep it for compatibility with old code here?). I was trying to make the new code match the surrounding code. I don't think the variable names matter very much here, but I'll change them if you feel strongly about it. >> +static HRESULT WINAPI FolderItemsImpl_Item(FolderItems3 *iface, VARIANT index, FolderItem **ppid) >> +{ >> + FIXME("(%p,%p)\n", iface, ppid); > > You forgot to trace the index parameter here. Good point. I'll fix that and resubmit the series (although hopefully Alexandre will have taken patch 1 without changes). >> @@ -1093,8 +1297,7 @@ static HRESULT WINAPI FolderImpl_Items(Folder3 *iface, FolderItems **ppid) >> { >> FIXME("(%p,%p)\n", iface, ppid); > > You forgot to change the FIXME to a TRACE. This is actually intentional, because the patch makes this function return a list of items, but the list is empty. I'm going to change the FIXME to a TRACE in the patch that populates the list: https://github.com/alexhenrie/wine/commits/master Thanks for the feedback. -Alex

July 4, 2016

Re: [PATCH v2 05/11] d3d11/tests: Add test for creating unordered access views.

by Henri Verbeet

On 4 July 2016 at 12:26, Józef Kucia <jkucia(a)codeweavers.com> wrote: > +static void test_create_unordered_access_view(void) > +{ > + static const D3D_FEATURE_LEVEL feature_level = D3D_FEATURE_LEVEL_11_0; > + D3D11_UNORDERED_ACCESS_VIEW_DESC uav_desc; > + D3D11_TEXTURE3D_DESC texture3d_desc; > + D3D11_TEXTURE2D_DESC texture2d_desc; > + ULONG refcount, expected_refcount; > + D3D11_SUBRESOURCE_DATA data = {0}; > + ID3D11UnorderedAccessView *uav; > + struct device_desc device_desc; > + D3D11_BUFFER_DESC buffer_desc; > + ID3D11Device *device, *tmp; > + ID3D11Texture3D *texture3d; > + ID3D11Texture2D *texture2d; > + ID3D11Resource *texture; > + ID3D11Buffer *buffer; > + IUnknown *iface; This is unused.

July 4, 2016

Re: [PATCH 2/5] shell32: Stub out FolderItems.

by Sebastian Lackner

On 04.07.2016 07:25, Alex Henrie wrote: > Cc: Christian Costa <titan.costa(a)gmail.com> > Cc: Sebastian Lackner <sebastian(a)fds-team.de> > > Wine Staging has included a similar patch since 1.7.51, however this > patch is not based on theirs. Actually the patch looks very similar, but this is probably caused by the fact that you both wrote it based on existing code. > > FolderItems2 worked in Windows 2000, but it was removed when > FolderItems3 was introduced in Windows XP. However, it doesn't hurt us > to provide this interface anyway for any old applications that might > want it. > > Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> > --- > dlls/shell32/shell32_main.h | 1 + > dlls/shell32/shelldispatch.c | 207 ++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 206 insertions(+), 2 deletions(-) > Some of the variable names (for example ppTInfo, rgszNames, rgDispId) sound a bit odd. Usually variable names without ugly prefix are preferred for new code (except we want to keep it for compatibility with old code here?). > +static HRESULT WINAPI FolderItemsImpl_Item(FolderItems3 *iface, VARIANT index, FolderItem **ppid) > +{ > + FIXME("(%p,%p)\n", iface, ppid); You forgot to trace the index parameter here. > + > + return E_NOTIMPL; > +} [...] > @@ -1093,8 +1297,7 @@ static HRESULT WINAPI FolderImpl_Items(Folder3 *iface, FolderItems **ppid) > { > FIXME("(%p,%p)\n", iface, ppid); You forgot to change the FIXME to a TRACE. > > - *ppid = NULL; > - return E_NOTIMPL; > + return FolderItems_Constructor(ppid); > } > > static HRESULT WINAPI FolderImpl_ParseName(Folder3 *iface, BSTR name, FolderItem **item) >

July 4, 2016

Re: PATCH] ws2_32: Allow user to enable IP dual stack.

by Marvin

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=23959 Your paranoid android. === build (build) === Patch failed to apply

July 4, 2016

Re: [PATCH 1/2] msvcr120: Add fegetenv implementation

by Marvin

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=23946 Your paranoid android. === w2008s64 (32 bit msvcr120) === msvcr120.c:706: Test failed: env.status = 0 === w2008s64 (64 bit msvcr120) === msvcr120.c:706: Test failed: env.status = 10

July 4, 2016

Re: [PATCH] advapi32: Add some tests for StartTrace(). (v2)

by Marvin

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=23944 Your paranoid android. === wxppro (32 bit eventlog) === eventlog.c:1224: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1235: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1264: Test failed: Expected success eventlog.c:1272: Test failed: Expected ERROR_ALREADY_EXISTS, got 161 === w2003std (32 bit eventlog) === eventlog.c:1224: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1235: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1264: Test failed: Expected success eventlog.c:1272: Test failed: Expected ERROR_ALREADY_EXISTS, got 161 === wvistau64 (32 bit eventlog) === eventlog.c:1224: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1235: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1264: Test failed: Expected success eventlog.c:1272: Test failed: Expected ERROR_ALREADY_EXISTS, got 161 === w2008s64 (32 bit eventlog) === eventlog.c:1224: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1235: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1264: Test failed: Expected success eventlog.c:1272: Test failed: Expected ERROR_ALREADY_EXISTS, got 161 === w7u (32 bit eventlog) === eventlog.c:1224: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1235: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1264: Test failed: Expected success eventlog.c:1272: Test failed: Expected ERROR_ALREADY_EXISTS, got 161 === w7pro64 (32 bit eventlog) === eventlog.c:1224: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1235: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1264: Test failed: Expected success eventlog.c:1272: Test failed: Expected ERROR_ALREADY_EXISTS, got 161 === w8 (32 bit eventlog) === eventlog.c:1224: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1235: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1264: Test failed: Expected success eventlog.c:1272: Test failed: Expected ERROR_ALREADY_EXISTS, got 161 === w864 (32 bit eventlog) === eventlog.c:1224: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1235: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1264: Test failed: Expected success eventlog.c:1272: Test failed: Expected ERROR_ALREADY_EXISTS, got 161 === w1064 (32 bit eventlog) === eventlog.c:1224: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1235: Test failed: Expected ERROR_INVALID_PARAMETER, got 161 eventlog.c:1264: Test failed: Expected success eventlog.c:1272: Test failed: Expected ERROR_ALREADY_EXISTS, got 161

July 4, 2016

Re: [PATCH] advapi32: Add some tests for StartTrace(). (v2)

by Austin English

On Jul 4, 2016 1:25 AM, "Zebediah Figura" <z.figura12(a)gmail.com> wrote: > > --- > dlls/advapi32/tests/eventlog.c | 139 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 139 insertions(+) > > diff --git a/dlls/advapi32/tests/eventlog.c b/dlls/advapi32/tests/eventlog.c > index 3ca59c3..5163537 100644 > --- a/dlls/advapi32/tests/eventlog.c > +++ b/dlls/advapi32/tests/eventlog.c > @@ -20,12 +20,15 @@ > > #include <stdarg.h> > > +#include "initguid.h" > #include "windef.h" > #include "winbase.h" > #include "winerror.h" > #include "winnt.h" > #include "winreg.h" > #include "sddl.h" > +#include "wmistr.h" > +#include "evntrace.h" > > #include "wine/test.h" > > @@ -1142,6 +1145,141 @@ static void cleanup_eventlog(void) > ok(bret, "Expected MoveFileEx to succeed: %d\n", GetLastError()); > } > > +static void test_trace(void) > +{ > + const char sessionname[] = "Wine"; > + const char filepath[] = "wine.etl"; > + const LONG lnoffset = ((sizeof(EVENT_TRACE_PROPERTIES) >> 3) + 1) << 3; //pad to 8 bytes > + const LONG lfnoffset = (((lnoffset + sizeof(sessionname)) >> 3) + 1) << 3; //pad to 8 bytes Please avoid C++ style comments, use /* this instead */

July 4, 2016

Re: [PATCH signed-off] secur32 : add and fix some algids

by Max Qian

Fixed the tabs and added CALG_ECDH_EPHEM in v2 of the patch. Can you look over it?

July 3, 2016

Re: [PATCH signed-off] secur32 : add and fix some algids

by Maxqia

Fixed the tabs and added CALG_ECDH_EPHEM in v2 of the patch. Can you look over it?

July 3, 2016

Re: [PATCH] wined3d/resource: Performance improvement

by Patrick Rudolph

On 2016-07-03 11:55 AM, Stefan Dösinger wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > Am 2016-07-03 um 10:04 schrieb Henri Verbeet: >> Finally, there the possibility that Windows filters invalid flag >> combinations like Wine currently does, and picking either DISCARD >> or NOOVERWRITE will break an application that depends on that. > When I originally added support for dynamic buffers in 2010 or so I > just forwarded the flags to the driver. That did break some > applications. I'll try to find out which one it was, I believe I put > some details in the patch description. > > It's also possible that there are some differences between textures > and buffers. In d3d <= 9, D3DLOCK_NOOVERWRITE is not supported on > textures. So it might be filtered out, and DISCARD prevails. But my > understanding is that Unigine uses the questionable combination on > buffers, not textures. > It uses both flags on buffers in d3d9 mode. I'm going to add tests for it. Regards, Patrick > > > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2 > > iQIcBAEBCAAGBQJXeOEWAAoJEN0/YqbEcdMwBRQP/jsjUZXuKbUeoh3iaorwY02B > qZXhtc2IbQlZt2989jAGIjuZoCtE2sP8oRkmtbGI3nfIthOxFKrwcPKnqYiz40aF > JqCqYiwMNSpcqDKEhC92w8ad4F8AMBIG/4IUVBtU/uL0W2fBKNJ0rKE4v2sNefo1 > OTa0ZCtUSCbvErO9xgen43KparIRNK1juxlkE7CVHkWwK3oBl8GO+IzJ6ZKSN54A > ftz48C7if3y0ul2Xs/HVC/J3UqnksgL1sRnxUeDrFC6MRsNQZaf0D3Gn9vUdqukt > t+hxArnt6DEgMiPlbh3Zhoso4lNzoJJxPpldmWYAgEiU5l6BqVnvSuDUzZZGjLqz > 8dg0QEzjDbta5rLwMi6NnZxTYinx8F61YMOUSfMIkKofz4Xc7v2TnEsgMRI9XuoV > f9KZVssvpYoBuQHI0qEpK4eEBqlU/ZtFh39O+j13980Z6wNtnCQVYUtMcY8hU8Nv > kf5iN/eOuMVrMV6EKqZswJe1CHfAfhVhLfD9kOMRREb5MjrAVyreigbHR/RXG3BH > K9bWwCnnaVfQDtJzUw9ab+e+uSbkkiymjrkWY8JKI3E0d8YBnupxS8GcwJTT0y0t > x5K/Fzp/a2U+x4FV8MZewIhT4azA+DEsgHi/6dviRi6/8ct6J4TTZZqCRKtgWFBy > O8waYy1TvNoBQETnhMaC > =7Pmz > -----END PGP SIGNATURE-----

July 3, 2016

Re: [PATCH] wined3d/resource: Performance improvement

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Am 2016-07-03 um 10:04 schrieb Henri Verbeet: > Finally, there the possibility that Windows filters invalid flag > combinations like Wine currently does, and picking either DISCARD > or NOOVERWRITE will break an application that depends on that. When I originally added support for dynamic buffers in 2010 or so I just forwarded the flags to the driver. That did break some applications. I'll try to find out which one it was, I believe I put some details in the patch description. It's also possible that there are some differences between textures and buffers. In d3d <= 9, D3DLOCK_NOOVERWRITE is not supported on textures. So it might be filtered out, and DISCARD prevails. But my understanding is that Unigine uses the questionable combination on buffers, not textures. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXeOEWAAoJEN0/YqbEcdMwBRQP/jsjUZXuKbUeoh3iaorwY02B qZXhtc2IbQlZt2989jAGIjuZoCtE2sP8oRkmtbGI3nfIthOxFKrwcPKnqYiz40aF JqCqYiwMNSpcqDKEhC92w8ad4F8AMBIG/4IUVBtU/uL0W2fBKNJ0rKE4v2sNefo1 OTa0ZCtUSCbvErO9xgen43KparIRNK1juxlkE7CVHkWwK3oBl8GO+IzJ6ZKSN54A ftz48C7if3y0ul2Xs/HVC/J3UqnksgL1sRnxUeDrFC6MRsNQZaf0D3Gn9vUdqukt t+hxArnt6DEgMiPlbh3Zhoso4lNzoJJxPpldmWYAgEiU5l6BqVnvSuDUzZZGjLqz 8dg0QEzjDbta5rLwMi6NnZxTYinx8F61YMOUSfMIkKofz4Xc7v2TnEsgMRI9XuoV f9KZVssvpYoBuQHI0qEpK4eEBqlU/ZtFh39O+j13980Z6wNtnCQVYUtMcY8hU8Nv kf5iN/eOuMVrMV6EKqZswJe1CHfAfhVhLfD9kOMRREb5MjrAVyreigbHR/RXG3BH K9bWwCnnaVfQDtJzUw9ab+e+uSbkkiymjrkWY8JKI3E0d8YBnupxS8GcwJTT0y0t x5K/Fzp/a2U+x4FV8MZewIhT4azA+DEsgHi/6dviRi6/8ct6J4TTZZqCRKtgWFBy O8waYy1TvNoBQETnhMaC =7Pmz -----END PGP SIGNATURE-----

July 3, 2016

Re: Re: [PATCH] wined3d/resource: Performance improvement

by Henri Verbeet

On 2 July 2016 at 08:49, Patrick Rudolph <siro(a)das-labor.org> wrote: >> It makes logical sense to me that mapping a buffer with both DISCARD and >> NOOVERWRITE would effectively ignore NOOVERWRITE. I've noticed that at >> least some engines will map a part of a buffer with NOOVERWRITE and draw >> with that part, map the next part of the buffer with NOOVERWRITE and >> draw with that part, etc, then after the last part of the buffer it maps >> with DISCARD to get new memory and keep going while the previous buffer >> memory is handled asynchronously by the driver or card. >> >> I notice that wined3d has some parts where it ignores/removes DISCARD, >> so it may not be a good idea to leave both DISCARD and NOOVERWRITE set. >> In such a case, the driver will get NOOVERWRITE for a part of the buffer >> that's still in use. It would probably be better to remove NOOVERWRITE >> when both DISCARD and NOOVERWRITE are set. > > Both flags are independent of each other and provide useful information > to the OpenGL driver. The driver knows best how to map the resource. > By passing NOOVERWRITE and DISCARD it's likely going to use NOOVERWRITE, > as it's even less overhead than DISCARD. > > I don't think that WINE should drop flags, except there's a driver bug > that can't be fixed (in the near future). > The patch is at the very least questionable. The way NOOVERWRITE/DISCARD are normally intended to be used is for streaming vertex data from the CPU to the GPU. NOOVERWRITE means the application promises it won't touch data that may be in use by the GPU, DISCARD means the application doesn't need the contents of the buffer any more, and it can go away after the GPU is done with it. In practice this means applications use NOOVERWRITE to append to the buffer until it's full, and then use DISCARD to get a new block of video memory. It's unclear what should happen when the application specifies both, but "have the driver pick one" seems unlikely to be what the application intended. In particular, if the application specified NOOVERWRITE because it determined (somehow, e.g. using fences) that it's not going to touch any data in use by the GPU, DISCARD is redundant and would lead to the driver doing unnecessary work if it picked DISCARD. On the other hand, if the application didn't make that determination and the driver picks NOOVERWRITE, the application is potentially going to write over data still in use by the GPU and cause geometry corruption. Finally, there the possibility that Windows filters invalid flag combinations like Wine currently does, and picking either DISCARD or NOOVERWRITE will break an application that depends on that. It could very well be the case that "DISCARD | NOOVERWRITE" behaves like e.g. just "DISCARD", but having the driver just pick one of them is almost certainly the wrong thing to do. Like so many other things, determining the correct behaviour would require some tests. This is of course one of the harder things to write tests for, but I don't think it's impossible.

July 3, 2016

Re: [PATCH] advapi32: Add some tests for StartTrace().

by GOUJON Alexandre

On 07/02/2016 10:57 PM, Zebediah Figura wrote: > On 07/02/2016 07:47 AM, Marvin wrote: >> 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=23920 >> >> Your paranoid android. >> >> >> === wxppro (32 bit eventlog) === >> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1185: Test failed: Expected ERROR_BAD_LENGTH, got 87 >> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1272: Test failed: Expected success >> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >> >> === w2003std (32 bit eventlog) === >> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1185: Test failed: Expected ERROR_BAD_LENGTH, got 87 >> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1272: Test failed: Expected success >> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >> >> === wvistau64 (32 bit eventlog) === >> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1272: Test failed: Expected success >> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >> >> === w2008s64 (32 bit eventlog) === >> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1272: Test failed: Expected success >> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >> >> === w7u (32 bit eventlog) === >> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1272: Test failed: Expected success >> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >> >> === w7pro64 (32 bit eventlog) === >> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1272: Test failed: Expected success >> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >> >> === w8 (32 bit eventlog) === >> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1272: Test failed: Expected success >> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >> >> === w864 (32 bit eventlog) === >> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1272: Test failed: Expected success >> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >> >> === w1064 (32 bit eventlog) === >> eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 >> eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 >> eventlog.c:1272: Test failed: Expected success >> eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 >> >> > This test is failing when built 32 bits, but I tried building both > 64-bit and 32-bit executables using mingw-w64 on my win7 machine, and > they both worked. I'm a newbie (this is my first patch), so I'm probably > doing something wrong, but I don't know what. The error in question (87) > is probably associated with the wrong sizeof() being returned, but I > don't know why that would happen. > > Thanks, > Zeb Welcome to wine ! Nothing really wrong at a first look but we can nitpick (inconsistent formatting, PFOO usage instead of FOO *, etc.). Returned error may depend on Windows version, installed Service Pack, dll version, structure version... To broaden your tests, you can use the test bot (https://wiki.winehq.org/Wine_TestBot) In your case, it may be an alignment issue (see http://stackoverflow.com/questions/9049971/windows-etw-starttrace-failing-w…)

July 3, 2016

Re: [PATCH] advapi32: Add some tests for StartTrace().

by Zebediah Figura

On 07/02/2016 07:47 AM, Marvin wrote: > 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=23920 > > Your paranoid android. > > > === wxppro (32 bit eventlog) === > eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1185: Test failed: Expected ERROR_BAD_LENGTH, got 87 > eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 > eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1272: Test failed: Expected success > eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 > > === w2003std (32 bit eventlog) === > eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1185: Test failed: Expected ERROR_BAD_LENGTH, got 87 > eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 > eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1272: Test failed: Expected success > eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 > > === wvistau64 (32 bit eventlog) === > eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 > eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1272: Test failed: Expected success > eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 > > === w2008s64 (32 bit eventlog) === > eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 > eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1272: Test failed: Expected success > eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 > > === w7u (32 bit eventlog) === > eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 > eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1272: Test failed: Expected success > eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 > > === w7pro64 (32 bit eventlog) === > eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 > eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1272: Test failed: Expected success > eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 > > === w8 (32 bit eventlog) === > eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 > eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1272: Test failed: Expected success > eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 > > === w864 (32 bit eventlog) === > eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 > eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1272: Test failed: Expected success > eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 > > === w1064 (32 bit eventlog) === > eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 > eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 > eventlog.c:1272: Test failed: Expected success > eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 > > This test is failing when built 32 bits, but I tried building both 64-bit and 32-bit executables using mingw-w64 on my win7 machine, and they both worked. I'm a newbie (this is my first patch), so I'm probably doing something wrong, but I don't know what. The error in question (87) is probably associated with the wrong sizeof() being returned, but I don't know why that would happen. Thanks, Zeb

July 2, 2016

Re: [PATCH 1/3] kernel32/tests/fiber: Add tests for fiber-local storage.

by André Hentschel

Am 02.07.2016 um 04:41 schrieb John Sheu: > Signed-off-by: John Sheu <sheu(a)google.com> Hi and welcome to Wine development! May I ask if you could name an application which will benefit from your fixes? Your tests currently fail on our testbot as you can see here: https://source.winehq.org/patches/ Some inline comments: > static VOID WINAPI FiberMainProc(LPVOID lpFiberParameter) > { > BYTE *tparam = (BYTE *)lpFiberParameter; > - cbCount++; > + fiberCount++; > ok(*tparam == 185, "Parameterdata expected not to be changed\n"); > + if (fls_index_to_set != FLS_OUT_OF_INDEXES) > + { > + if (!pFlsSetValue || !pFlsGetValue) > + { > + win_skip( "Fiber Local Storage not supported\n" ); > + } > + else > + { > + PVOID pret; > + BOOL bret; > + > + pret = FlsGetValue(fls_index_to_set); > + ok(pret == NULL, "FlsGetValue returned %p, expected NULL\n", pret); > + > + /* Set the FLS value */ > + SetLastError( 0xdeadbeef ); > + bret = pFlsSetValue(fls_index_to_set, fls_value_to_set); > + ok(bret, "FlsSetValue failed with error %u\n", GetLastError()); > + > + /* Verify that FlsGetValue retrieves the value set by FlsSetValue */ > + SetLastError( 0xdeadbeef ); > + pret = FlsGetValue(fls_index_to_set); > + ok(pret == fls_value_to_set, > + "FlsGetValue returned %p, expected %p\n", pret, fls_value_to_set); > + ok(GetLastError() == ERROR_SUCCESS, > + "FlsGetValue error %u\n", GetLastError()); > + } > + } > pSwitchToFiber(fibers[0]); > } You are mixing tabs and spaces in FiberMainProc, spaces are preferred. Further you can avoid some line brakes in ok() calls, only wrap at around 100 > +static void test_FiberLocalStorageWithFibers(PFLS_CALLBACK_FUNCTION cbfunc) { > + PVOID val1 = (PVOID) 0x314; > + PVOID val2 = (PVOID) 0x152; > + BOOL ret; > + > + if (!pConvertThreadToFiber || !pSwitchToFiber || !pDeleteFiber) > + { > + win_skip( "Fibers not supported\n" ); > + return; > + } > + if (!pFlsAlloc || !pFlsFree || !pFlsSetValue || !pFlsGetValue) > + { > + win_skip( "Fiber Local Storage not supported\n" ); > + return; > + } > + > + > + fls_index_to_set = pFlsAlloc(cbfunc); > + ok(fls_index_to_set != FLS_OUT_OF_INDEXES, "FlsAlloc failed with error %d\n", GetLastError()); > + > + fiberCount = 0; > + cbCount = 0; > + fibers[0] = pConvertThreadToFiber(&testparam); Why do you convert the current thread into a fiber? It seems to me that is causing the crash on w2k3. You could try to run that test in a seperate thread > + fibers[1] = pCreateFiber(0,FiberMainProc,&testparam); > + fibers[2] = pCreateFiber(0,FiberMainProc,&testparam); > + ok(fibers[1] != 0, "CreateFiber failed with error %d\n", GetLastError()); > + ok(fibers[2] != 0, "CreateFiber failed with error %d\n", GetLastError()); > + ok(fiberCount == 0, "Wrong fiber count: %d\n", fiberCount); > + ok(cbCount == 0, "Wrong callback count: %d\n", cbCount); > + > + fiberCount = 0; > + cbCount = 0; > + fls_value_to_set = val1; > + pSwitchToFiber(fibers[1]); > + ok(fiberCount == 1, "Wrong fiber count: %d\n", fiberCount); > + ok(cbCount == 0, "Wrong callback count: %d\n", cbCount); > + > + fiberCount = 0; > + cbCount = 0; > + fls_value_to_set = val2; > + pSwitchToFiber(fibers[2]); > + ok(fiberCount == 1, "Wrong fiber count: %d\n", fiberCount); > + ok(cbCount == 0, "Wrong callback count: %d\n", cbCount); > + > + fls_value_to_set = val2; > + ret = pFlsSetValue(fls_index_to_set, fls_value_to_set); > + ok(ret, "FlsSetValue failed\n"); > + ok(val2 == pFlsGetValue(fls_index_to_set), "FlsGetValue failed\n"); > + > + fiberCount = 0; > + cbCount = 0; > + fls_value_to_set = val1; > + pDeleteFiber(fibers[1]); > + ok(fiberCount == 0, "Wrong fiber count: %d\n", fiberCount); > + todo_wine > + { > + ok(cbCount == 1, "Wrong callback count: %d\n", cbCount); > + } You can make that a one liner: todo_wine ok(); Hope that helps Have fun!

July 2, 2016

Re: [PATCH] advapi32: Add some tests for StartTrace().

by Marvin

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=23920 Your paranoid android. === wxppro (32 bit eventlog) === eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1185: Test failed: Expected ERROR_BAD_LENGTH, got 87 eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1272: Test failed: Expected success eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 === w2003std (32 bit eventlog) === eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1185: Test failed: Expected ERROR_BAD_LENGTH, got 87 eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1272: Test failed: Expected success eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 === wvistau64 (32 bit eventlog) === eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1272: Test failed: Expected success eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 === w2008s64 (32 bit eventlog) === eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1272: Test failed: Expected success eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 === w7u (32 bit eventlog) === eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1272: Test failed: Expected success eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 === w7pro64 (32 bit eventlog) === eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1272: Test failed: Expected success eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 === w8 (32 bit eventlog) === eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1272: Test failed: Expected success eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 === w864 (32 bit eventlog) === eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1272: Test failed: Expected success eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87 === w1064 (32 bit eventlog) === eventlog.c:1172: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1194: Test failed: Expected ERROR_BAD_LENGTH, got 87 eventlog.c:1255: Test failed: Expected ERROR_BAD_PATHNAME, got 87 eventlog.c:1272: Test failed: Expected success eventlog.c:1280: Test failed: Expected ERROR_ALREADY_EXISTS, got 87

July 2, 2016