- wine-devel - winehq.org

Re: [PATCH 1/2] advapi32: Added TokenIsAppContainer tests.
by Marvin 15 Nov '16

15 Nov '16

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=26664 Your paranoid android. === w1064 (32 bit security) === security.c:4036: Test failed: Builtin Users ACE has unexpected flags (0x12 != 0xa) security.c:4040: Test failed: Builtin Users ACE has unexpected mask (0x20019 != 0x80000000) security.c:4049: Test failed: Builtin Admins ACE has unexpected flags (0x12 != 0x0) === w1064 (64 bit security) === security.c:4036: Test failed: Builtin Users ACE has unexpected flags (0x2 != 0xa) security.c:4040: Test failed: Builtin Users ACE has unexpected mask (0x20019 != 0x80000000) security.c:4049: Test failed: Builtin Admins ACE has unexpected flags (0x2 != 0x0)

1 0

Re: [PATCH] winegstreamer: Fix memory leak in amt_from_gst_caps_video.
by Andrew Eikum 15 Nov '16

15 Nov '16

This is an improvement, but I think we should go further and fix leaks in all of the failure cases. The problem is we're using the amt_from_gst_caps_video return value inconsistently. In accept_caps_sink, we unconditionally free the resulting AM_MEDIA_TYPE. But in setcaps_sink, we only free it, via the pin->pmt assignment, if amt_from_gst_caps_video succeeds. I think we should always free the allocated memory in the failure cases in amt_from_gst_caps_video, and then only free the resulting AMT in the calling functions if amt_from_gst_caps_video succeeded. The resulting AMT should never be used or freed if amt_from_gst_caps_video fails. Note that amt_from_gst_caps_audio has the same inconsistent return type handling. I would accept a separate patch to fix this handling, too. (Unlike for video, it doesn't currently matter in practice because of amt_from_gst_caps_audio's implementation.) Does that make sense? Andrew On Mon, Nov 14, 2016 at 11:43:21PM -0700, Alex Henrie wrote: > Cc: Andrew Eikum <aeikum(a)codeweavers.com> > > Coverity #713781, "Variable vih going out of scope leaks the storage it > points to." > > Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> > --- > dlls/winegstreamer/gstdemux.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c > index a6a41ab..5d97510 100644 > --- a/dlls/winegstreamer/gstdemux.c > +++ b/dlls/winegstreamer/gstdemux.c > @@ -179,18 +179,21 @@ static gboolean amt_from_gst_caps_audio(GstCaps *caps, AM_MEDIA_TYPE *amt) > > static gboolean amt_from_gst_caps_video(GstCaps *caps, AM_MEDIA_TYPE *amt) > { > - VIDEOINFOHEADER *vih = CoTaskMemAlloc(sizeof(*vih)); > - BITMAPINFOHEADER *bih = &vih->bmiHeader; > + VIDEOINFOHEADER *vih; > + BITMAPINFOHEADER *bih; > gint32 width = 0, height = 0, nom = 0, denom = 0; > GstVideoInfo vinfo; > > if (!gst_video_info_from_caps (&vinfo, caps)) > return FALSE; > width = vinfo.width; > height = vinfo.height; > nom = vinfo.fps_n; > denom = vinfo.fps_d; > > + vih = CoTaskMemAlloc(sizeof(*vih)); > + bih = &vih->bmiHeader; > + > amt->formattype = FORMAT_VideoInfo; > amt->pbFormat = (BYTE*)vih; > amt->cbFormat = sizeof(*vih); > -- > 2.10.2 >

2 1

Re: [resend PATCH 2/4] oledb32: IErrorRecords uses 0-based index to access record info
by Alistair Leslie-Hughes 15 Nov '16

15 Nov '16

Hi Nikolay, On 15/11/16 03:56, Nikolay Sivov wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > > + hr = IErrorRecords_GetRecordCount(errrecs, &cnt); > + ok(hr == S_OK, "got %08x\n", hr); > + ok(cnt == 0, "expected 2 got %d\n", cnt); The comment here is incorrect. Best Regards Alistair.

2 1

Re: [PATCH] cabinet: Properly initialize our internal fci structure v2(bug 36115)(valgrind)
by Huw Davies 14 Nov '16

14 Nov '16

On 14 Nov 2016, at 16:46, Detlef Riekenberg <wine.dev(a)web.de> wrote: > > We did not initialize all items for our internal fci structure. > This should fix bug 36115 > > v2: > Remove comments, as suggested by Huw > > -- > bye bye ... Detlef > > Signed-off-by: Detlef Riekenberg <wine.dev(a)web.de> > --- > dlls/cabinet/fci.c | 19 +------------------ > 1 file changed, 1 insertion(+), 18 deletions(-) > > diff --git a/dlls/cabinet/fci.c b/dlls/cabinet/fci.c > index 70d4f3f..03fbf46 100644 > --- a/dlls/cabinet/fci.c > +++ b/dlls/cabinet/fci.c > @@ -1036,6 +1036,7 @@ HFCI __cdecl FCICreate( > return NULL; > } > > + memset(p_fci_internal, 0, sizeof(FCI_Int)); I also suggested changing the sizeof(FCI_Int) -> sizeof(*p_fci_internal), but maybe that wasn’t very clear. Huw.

1 0

WineConf decisions
by Alexandre Julliard 14 Nov '16

14 Nov '16

At this year's WineConf we again had good constructive discussions about the development process, and decided a number of things: - Version numbering will change to a sligthly shorter scheme. The next stable release will be called 2.0. Stable release updates will follow the existing scheme: 2.0.1, 2.0.2, etc. For the developemnt branch we will drop the intermediate number: releases will be numbered 2.1, 2.2, 2.3, etc. and the next stable release in late 2017 will be called 3.0. - Code freeze for the 2.0 release will start after 1.9.24, in 3-4 weeks, with a final release targeted around the end of the year. - Michael Stefaniuc has agreed to continue maintaining the stable branch, and will be in charge of the 2.0.x releases. - Since it sometimes takes months for the stable release to get into distributions, we'll look into providing binary packages of the stable branch to make it easier for users to stay up to date. - The default reported Windows version will be upgraded to Windows 7. This will be done on the development branch soon after 2.0 is released, so that we have the time to catch regressions and address them before the next stable release. - To further the integration of WineHQ and Wine-Staging, we'll look into merging the Staging website into the Wine Wiki. We'll also merge the IRC channels, and encourage people to discuss Staging issues on #winehq and #winehackers, with #wine-staging being retired at some point. - To make sure that no patch falls off the list unreviewed, we are going to implement a mechanism to automatically assign a reviewer from the MAINTAINERS pool when a patch has been in New state for a week without action. - Since reviewers assigned at random may not always be comfortable with fully signing off on a patch, we discussed adding an Acked-by header as a weaker version of Signed-off-by. We finally decided that for now, an informal mail to wine-devel would be good enough. We may revisit that decision if a formal header makes automation easier. I'm hoping that these improvements will make things better for everybody. As always, feedback is encouraged, especially from people who couldn't make it to WineConf. As we implement these changes, please let us know how we are doing, and where we can do better. -- Alexandre Julliard julliard(a)winehq.org

1 0

AppDB Maintainership and Updates
by Jeremy Newman 14 Nov '16

14 Nov '16

Hey all! Just wanted to let you all know that at WineConf we decided that I should once again take up the mantle of AppDB codebase maintainer. Woo! I'll be in charge of reviewing and approving AppDB patches from now on. If you want to submit patches, please send them to the wine-patches mailing list with the subject prefix of [AppDB]. Bugs and feature requests should be filed on Bugzilla as usual. Discussion can happen here on wine-devel, IRC #winehackers, or preferably on the Forums in the new "Website Issues" forum. I've started the process of cleaning up some of the code and moving things around. Please bear with me as I'll be doing this in whatever spare time I can find. I created a ToDo page in the Wiki that will track the various things that I've agreed to work on. If it is not listed there, then it is open for someone else to jump on. https://wiki.winehq.org/AppDB_ToDo_List -N

1 0

Re: [17/23] wined3d: Implement return instruction.
by Matteo Bruni 14 Nov '16

14 Nov '16

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > @@ -6079,8 +6101,26 @@ static GLuint shader_glsl_generate_vshader(const struct wined3d_context *context > /* Base Shader Body */ > shader_generate_main(shader, buffer, reg_maps, function, &priv_ctx); > > + shader_addline(buffer, "}\n"); > + > + TRACE("Compiling shader object %u.\n", shader_id); > + shader_glsl_compile(gl_info, shader_id, buffer->buffer); > + > + return shader_id; > +} This breaks < SM4 shaders, those don't have an explicit return instruction at the end. In general, each patch should pass the wine tests. Please run the d3d tests and make sure that none are broken by your patchset. Specifically, this patch breaks the d3d9 visual test pretty badly. It looks like there are more failures introduced by other patches in the series but I haven't tracked them down. > +static void shader_glsl_prologue_vs(const struct wined3d_shader_context *ctx) I think this should actually be "epilogue".

1 0

Re: [11/23] wined3d: Implement primitive restart index.
by Matteo Bruni 14 Nov '16

14 Nov '16

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > Signed-off-by: Andrew Wesie <awesie(a)gmail.com> > --- > dlls/wined3d/directx.c | 3 +++ > dlls/wined3d/glsl_shader.c | 14 ++++++++++++++ > dlls/wined3d/wined3d_gl.h | 2 ++ > 3 files changed, 19 insertions(+) > > diff --git a/dlls/wined3d/directx.c b/dlls/wined3d/directx.c > index a8c8c9f..c930cf6 100644 > --- a/dlls/wined3d/directx.c > +++ b/dlls/wined3d/directx.c > @@ -121,6 +121,7 @@ static const struct wined3d_extension_map gl_extension_map[] = > {"GL_ARB_draw_elements_base_vertex", ARB_DRAW_ELEMENTS_BASE_VERTEX }, > {"GL_ARB_draw_instanced", ARB_DRAW_INSTANCED }, > {"GL_ARB_ES2_compatibility", ARB_ES2_COMPATIBILITY }, > + {"GL_ARB_ES3_compatibility", ARB_ES3_COMPATIBILITY }, > {"GL_ARB_explicit_attrib_location", ARB_EXPLICIT_ATTRIB_LOCATION }, > {"GL_ARB_fragment_coord_conventions", ARB_FRAGMENT_COORD_CONVENTIONS}, > {"GL_ARB_fragment_program", ARB_FRAGMENT_PROGRAM }, > @@ -228,6 +229,7 @@ static const struct wined3d_extension_map gl_extension_map[] = > {"GL_NV_half_float", NV_HALF_FLOAT }, > {"GL_NV_light_max_exponent", NV_LIGHT_MAX_EXPONENT }, > {"GL_NV_point_sprite", NV_POINT_SPRITE }, > + {"GL_NV_primitive_restart", NV_PRIMITIVE_RESTART }, > {"GL_NV_register_combiners", NV_REGISTER_COMBINERS }, > {"GL_NV_register_combiners2", NV_REGISTER_COMBINERS2 }, > {"GL_NV_texgen_reflection", NV_TEXGEN_REFLECTION }, > @@ -3125,6 +3127,7 @@ static void load_gl_funcs(struct wined3d_gl_info *gl_info) > USE_GL_FUNC(glMapBuffer) /* OpenGL 1.5 */ > USE_GL_FUNC(glPointParameteri) /* OpenGL 1.4 */ > USE_GL_FUNC(glPointParameteriv) /* OpenGL 1.4 */ > + USE_GL_FUNC(glPrimitiveRestartIndex) > USE_GL_FUNC(glShaderSource) /* OpenGL 2.0 */ > USE_GL_FUNC(glStencilFuncSeparate) /* OpenGL 2.0 */ > USE_GL_FUNC(glStencilOpSeparate) /* OpenGL 2.0 */ AFAICS this function was added in OpenGL 3.1 and there is no specific ARB extension for it. Maybe adding a new WINED3D_GL_ entry to flag the availability of the function is a good idea. Also you probably want to MAP_GL_FUNCTION() the NV variant to the core one and add an entry for GL_ARB_ES3_compatibility to core_extensions. > diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c > index f136536..27a5533 100644 > --- a/dlls/wined3d/glsl_shader.c > +++ b/dlls/wined3d/glsl_shader.c > @@ -8613,6 +8613,20 @@ static void shader_glsl_init_context_state(struct wined3d_context *context) > > gl_info->gl_ops.gl.p_glEnable(GL_PROGRAM_POINT_SIZE); > checkGLcall("GL_PROGRAM_POINT_SIZE"); > + if (gl_info->supported[ARB_ES3_COMPATIBILITY]) > + { > + /* We prefer this method because it correctly handles 16-bit and 32-bit indices. */ > + gl_info->gl_ops.gl.p_glEnable(GL_PRIMITIVE_RESTART_FIXED_INDEX); > + checkGLcall("GL_PRIMITIVE_RESTART_FIXED_INDEX"); > + } > + else if (gl_info->supported[NV_PRIMITIVE_RESTART] && GL_EXTCALL(glPrimitiveRestartIndex)) > + { > + /* FIXME: Does this handle 16-bit indices correctly? */ > + GL_EXTCALL(glPrimitiveRestartIndex(0xFFFFFFFF)); > + checkGLcall("glPrimitiveRestartIndex"); > + gl_info->gl_ops.gl.p_glEnable(GL_PRIMITIVE_RESTART); > + checkGLcall("GL_PRIMITIVE_RESTART"); Probably not, but nothing prevents you to use 0xffff for 16-bit indices and force re-setting the restart index when changing between 16-bit and 32-bit indices.

2 2

Re: [08/23] wined3d: Allow viewport to be out-of-bounds if using offscreen target.
by Matteo Bruni 14 Nov '16

14 Nov '16

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > Signed-off-by: Andrew Wesie <awesie(a)gmail.com> > --- > dlls/wined3d/state.c | 42 ++++++++++++++++++++++-------------------- > 1 file changed, 22 insertions(+), 20 deletions(-) > > diff --git a/dlls/wined3d/state.c b/dlls/wined3d/state.c > index cd3763c..ed20e325 100644 > --- a/dlls/wined3d/state.c > +++ b/dlls/wined3d/state.c > @@ -4617,26 +4617,6 @@ static void viewport_miscpart(struct wined3d_context *context, const struct wine > struct wined3d_viewport vp = state->viewport; > unsigned int width, height; > > - if (target) > - { > - if (vp.width > target->width) > - vp.width = target->width; > - if (vp.height > target->height) > - vp.height = target->height; > - > - wined3d_rendertarget_view_get_drawable_size(target, context, &width, &height); > - } > - else if (depth_stencil) > - { > - width = depth_stencil->width; > - height = depth_stencil->height; > - } > - else > - { > - FIXME("No attachments draw calls not supported.\n"); > - return; > - } > - > gl_info->gl_ops.gl.p_glDepthRange(vp.min_z, vp.max_z); > checkGLcall("glDepthRange"); > /* Note: GL requires lower left, DirectX supplies upper left. This is > @@ -4644,7 +4624,29 @@ static void viewport_miscpart(struct wined3d_context *context, const struct wine > if (context->render_offscreen) > gl_info->gl_ops.gl.p_glViewport(vp.x, vp.y, vp.width, vp.height); > else > + { > + if (target) > + { > + if (vp.width > target->width) > + vp.width = target->width; > + if (vp.height > target->height) > + vp.height = target->height; > + > + wined3d_rendertarget_view_get_drawable_size(target, context, &width, &height); > + } > + else if (depth_stencil) > + { > + width = depth_stencil->width; > + height = depth_stencil->height; > + } > + else > + { > + FIXME("No attachments draw calls not supported.\n"); > + return; > + } > + > gl_info->gl_ops.gl.p_glViewport(vp.x, (height - (vp.y + vp.height)), vp.width, vp.height); > + } > checkGLcall("glViewport"); > } You need to make the same change to viewport_miscpart_cc() (you can blame me for that). Again, it would be very nice to have a testcase for this. Extending viewport_test() in d3d9/tests/visual.c and adding similar tests to the other d3d versions shouldn't be too hard.

2 2

Re: [PATCH] hidclass: All reports read or written to user space lead with a reportId
by Sebastian Lackner 14 Nov '16

14 Nov '16

On 14.11.2016 14:35, Aric Stewart wrote: > This is a little confusing because reports to and from the device > itself only have a leading reportId if the reportid is not 0. > Otherwise there is no leading reportId. So we need to add a 0 reportId > to reports coming from such a device and strip the leading 0 from > reports written to the device. > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/hidclass.sys/descriptor.c | 31 ++++----------- > dlls/hidclass.sys/device.c | 90 ++++++++++++++++++++++++++++-------------- > 2 files changed, 67 insertions(+), 54 deletions(-) > > > > 0001-hidclass-All-reports-read-or-written-to-user-space-lea.txt > > > diff --git a/dlls/hidclass.sys/descriptor.c b/dlls/hidclass.sys/descriptor.c > index 9a26f0f4..8bcda43 100644 > --- a/dlls/hidclass.sys/descriptor.c > +++ b/dlls/hidclass.sys/descriptor.c > @@ -887,10 +887,8 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > new_report(wine_report, input_features[0]); > data->dwInputReportCount++; > > - if (input_features[0]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + /* Room for the reportID */ > + bitOffset = 8; > > for (i = 0; i < i_count; i++) > { > @@ -901,10 +899,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > new_report(wine_report, input_features[i]); > data->dwInputReportCount++; > bitLength = max(bitOffset, bitLength); > - if (input_features[i]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + bitOffset = 8; > } > build_elements(wine_report, input_features[i], &bitOffset); > count_elements(input_features[i], &data->caps.NumberInputButtonCaps, > @@ -922,10 +917,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > data->dwOutputReportOffset = (BYTE*)wine_report - (BYTE*)data->InputReports; > new_report(wine_report, output_features[0]); > data->dwOutputReportCount++; > - if (output_features[0]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + bitOffset = 8; > > for (i = 0; i < o_count; i++) > { > @@ -936,10 +928,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > new_report(wine_report, output_features[i]); > data->dwOutputReportCount++; > bitLength = max(bitOffset, bitLength); > - if (output_features[0]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + bitOffset = 8; > } > build_elements(wine_report, output_features[i], &bitOffset); > count_elements(output_features[i], &data->caps.NumberOutputButtonCaps, > @@ -957,10 +946,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > data->dwFeatureReportOffset = (BYTE*)wine_report - (BYTE*)data->InputReports; > new_report(wine_report, feature_features[0]); > data->dwFeatureReportCount++; > - if (feature_features[0]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + bitOffset = 8; > > for (i = 0; i < f_count; i++) > { > @@ -971,10 +957,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( > new_report(wine_report, feature_features[i]); > data->dwFeatureReportCount++; > bitLength = max(bitOffset, bitLength); > - if (feature_features[0]->caps.ReportID != 0) > - bitOffset = 8; > - else > - bitOffset = 0; > + bitOffset = 8; > } > build_elements(wine_report, feature_features[i], &bitOffset); > count_elements(feature_features[i], &data->caps.NumberFeatureButtonCaps, > diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c > index 5dd5ebb..6454b67 100644 > --- a/dlls/hidclass.sys/device.c > +++ b/dlls/hidclass.sys/device.c > @@ -197,6 +197,28 @@ void HID_DeleteDevice(HID_MINIDRIVER_REGISTRATION *driver, DEVICE_OBJECT *device > IoDeleteDevice(device); > } > > +static NTSTATUS copy_packet_into_buffer(HID_XFER_PACKET *packet, BYTE* buffer, ULONG buffer_length, ULONG *out_length) > +{ > + *out_length = 0; > + if (buffer_length > packet->reportBufferLen) > + { > + if (packet->reportId != 0) > + { > + memcpy(buffer, packet->reportBuffer, packet->reportBufferLen); > + *out_length = packet->reportBufferLen; The length check is a bit too strict in this case. You are returning STATUS_BUFFER_OVERFLOW even when the buffer is sufficient (buffer_length == packet->reportBufferLen). > + } > + else > + { > + buffer[0] = 0; > + memcpy(&buffer[1], packet->reportBuffer, packet->reportBufferLen); > + *out_length = packet->reportBufferLen + 1; > + } > + return STATUS_SUCCESS; > + } > + else > + return STATUS_BUFFER_OVERFLOW; > +} > + > static void HID_Device_processQueue(DEVICE_OBJECT *device) > { > LIST_ENTRY *entry; > @@ -217,20 +239,14 @@ static void HID_Device_processQueue(DEVICE_OBJECT *device) > RingBuffer_Read(ext->ring_buffer, ptr, packet, &buffer_size); > if (buffer_size) > { > + NTSTATUS hr; Maybe its just me, but I think "hr" is really a very bad name for NTSTATUS codes. ;) > + ULONG out_length; > IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); > packet->reportBuffer = (BYTE *)packet + sizeof(*packet); > TRACE_(hid_report)("Processing Request (%i)\n",ptr); > - if (irpsp->Parameters.Read.Length >= packet->reportBufferLen) > - { > - memcpy(irp->AssociatedIrp.SystemBuffer, packet->reportBuffer, packet->reportBufferLen); > - irp->IoStatus.Information = packet->reportBufferLen; > - irp->IoStatus.u.Status = STATUS_SUCCESS; > - } > - else > - { > - irp->IoStatus.Information = 0; > - irp->IoStatus.u.Status = STATUS_BUFFER_OVERFLOW; > - } > + hr = copy_packet_into_buffer(packet, irp->AssociatedIrp.SystemBuffer, irpsp->Parameters.Read.Length, &out_length); > + irp->IoStatus.u.Status = hr; > + irp->IoStatus.Information = out_length; > } > else > { > @@ -331,7 +347,10 @@ static DWORD CALLBACK hid_device_thread(void *args) > if (!exit_now && irp->IoStatus.u.Status == STATUS_SUCCESS) > { > packet->reportBufferLen = irp->IoStatus.Information; > - packet->reportId = packet->reportBuffer[0]; > + if (ext->preparseData->InputReports[0].reportID) > + packet->reportId = packet->reportBuffer[0]; > + else > + packet->reportId = 0; > RingBuffer_Write(ext->ring_buffer, packet); > HID_Device_processQueue(device); > } > @@ -461,9 +480,17 @@ static NTSTATUS HID_set_to_device(DEVICE_OBJECT *device, IRP *irp) > NTSTATUS rc; > > TRACE_(hid_report)("Device %p Buffer length %i Buffer %p\n", device, irpsp->Parameters.DeviceIoControl.InputBufferLength, irp->AssociatedIrp.SystemBuffer); > - packet.reportBuffer = irp->AssociatedIrp.SystemBuffer; > - packet.reportId = ((char*)irp->AssociatedIrp.SystemBuffer)[0]; > - packet.reportBufferLen = irpsp->Parameters.DeviceIoControl.InputBufferLength; > + packet.reportId = ((BYTE*)irp->AssociatedIrp.SystemBuffer)[0]; > + if (packet.reportId == 0) > + { > + packet.reportBuffer = &((BYTE*)irp->AssociatedIrp.SystemBuffer)[1]; > + packet.reportBufferLen = irpsp->Parameters.DeviceIoControl.InputBufferLength - 1; > + } > + else > + { > + packet.reportBuffer = irp->AssociatedIrp.SystemBuffer; > + packet.reportBufferLen = irpsp->Parameters.DeviceIoControl.InputBufferLength; > + } > TRACE_(hid_report)("(id %i, len %i buffer %p)\n", packet.reportId, packet.reportBufferLen, packet.reportBuffer); > > rc = call_minidriver(irpsp->Parameters.DeviceIoControl.IoControlCode, > @@ -636,20 +663,15 @@ NTSTATUS WINAPI HID_Device_read(DEVICE_OBJECT *device, IRP *irp) > if (buffer_size) > { > IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp ); > + NTSTATUS hr; Same here. > + ULONG out_length; > packet->reportBuffer = (BYTE *)packet + sizeof(*packet); > TRACE_(hid_report)("Got Packet %p %i\n", packet->reportBuffer, packet->reportBufferLen); > - if (irpsp->Parameters.Read.Length >= packet->reportBufferLen) > - { > - memcpy(irp->AssociatedIrp.SystemBuffer, packet->reportBuffer, packet->reportBufferLen); > - irp->IoStatus.Information = packet->reportBufferLen; > - irp->IoStatus.u.Status = STATUS_SUCCESS; > - } > - else > - { > - irp->IoStatus.Information = 0; > - irp->IoStatus.u.Status = STATUS_BUFFER_OVERFLOW; > - } > - IoCompleteRequest( irp, IO_NO_INCREMENT ); > + > + hr = copy_packet_into_buffer(packet, irp->AssociatedIrp.SystemBuffer, irpsp->Parameters.Read.Length, &out_length); > + irp->IoStatus.Information = out_length; > + irp->IoStatus.u.Status = hr; > + IoCompleteRequest(irp, IO_NO_INCREMENT); > } > else > { > @@ -671,9 +693,17 @@ NTSTATUS WINAPI HID_Device_write(DEVICE_OBJECT *device, IRP *irp) > irp->IoStatus.Information = 0; > > TRACE_(hid_report)("Device %p Buffer length %i Buffer %p\n", device, irpsp->Parameters.Write.Length, irp->AssociatedIrp.SystemBuffer); > - packet.reportBuffer = irp->AssociatedIrp.SystemBuffer; > - packet.reportId = ((char*)irp->AssociatedIrp.SystemBuffer)[0]; > - packet.reportBufferLen = irpsp->Parameters.Write.Length; > + packet.reportId = ((BYTE*)irp->AssociatedIrp.SystemBuffer)[0]; > + if (packet.reportId == 0) > + { > + packet.reportBuffer = &((BYTE*)irp->AssociatedIrp.SystemBuffer)[1]; > + packet.reportBufferLen = irpsp->Parameters.Write.Length - 1; Besides the fact that the code looks suspicious (couldn't a 0 byte also have a different meaning?) this will probably break the udev backend. As documented in https://www.kernel.org/doc/Documentation/hid/hidraw.txt, the write() command expects that the first byte is 0 if the device does not use numbered reports. > + } > + else > + { > + packet.reportBuffer = irp->AssociatedIrp.SystemBuffer; > + packet.reportBufferLen = irpsp->Parameters.Write.Length; > + } > TRACE_(hid_report)("(id %i, len %i buffer %p)\n", packet.reportId, packet.reportBufferLen, packet.reportBuffer); > > rc = call_minidriver(IOCTL_HID_WRITE_REPORT, device, NULL, 0, &packet, sizeof(packet)); > > >

2 4

Re: [12/23] wined3d: Clear WINED3D_BUFFER_DISCARD flag in unmap.
by Henri Verbeet 14 Nov '16

14 Nov '16

On 13 November 2016 at 19:35, Andrew Wesie <awesie(a)gmail.com> wrote: > @@ -1248,6 +1248,7 @@ static void wined3d_buffer_unmap(struct wined3d_buffer *buffer) > context_release(context); > > buffer_clear_dirty_areas(buffer); > + buffer->flags &= ~WINED3D_BUFFER_DISCARD; Does filtering redundant WINED3D_MAP_DISCARD maps still work after this? Why is this change needed?

1 0

Re: [10/23] d3d11: Reset base vertex index to zero when not using indexed draw.
by Henri Verbeet 14 Nov '16

14 Nov '16

On 13 November 2016 at 19:35, Andrew Wesie <awesie(a)gmail.com> wrote: > Signed-off-by: Andrew Wesie <awesie(a)gmail.com> > --- > dlls/d3d11/device.c | 4 ++++ > 1 file changed, 4 insertions(+) > I think this shouldn't be needed when you do the state invalidation in wined3d_cs_exec_draw(). Ideally we'd probably just get rid of wined3d_device_set_base_vertex_index(), but d3d8 stateblocks complicate that.

1 0

Re: [09/23] wined3d: Fix mapping of SV_VertexID to gl_VertexID.
by Henri Verbeet 14 Nov '16

14 Nov '16

On 13 November 2016 at 19:35, Andrew Wesie <awesie(a)gmail.com> wrote: > @@ -481,7 +481,7 @@ void wined3d_cs_emit_draw(struct wined3d_cs *cs, int base_vertex_idx, unsigned i > > op = cs->ops->require_space(cs, sizeof(*op)); > op->opcode = WINED3D_CS_OP_DRAW; > - op->base_vertex_idx = base_vertex_idx; > + op->base_vertex_idx = indexed ? base_vertex_idx : 0; This change seems questionable. Why is it needed? > @@ -1988,7 +1988,11 @@ void CDECL wined3d_device_set_base_vertex_index(struct wined3d_device *device, I > { > TRACE("device %p, base_index %d.\n", device, base_index); > > - device->update_state->base_vertex_index = base_index; > + if (device->update_state->base_vertex_index != base_index) > + { > + device->update_state->base_vertex_index = base_index; > + device_invalidate_state(device, STATE_BASEVERTEXINDEX); > + } > } This should happen in wined3d_cs_exec_draw(). (The background is that we're working on executing that on a different thread, and state invalidation should generally happen in that same thread.) > @@ -1424,6 +1425,12 @@ static void shader_glsl_load_constants(void *shader_priv, struct wined3d_context > checkGLcall("glUniform4fv"); > } > > + if (update_mask & WINED3D_SHADER_CONST_BASE_VERTEX) > + { > + GL_EXTCALL(glUniform1i(prog->vs.base_vertex_location, state->base_vertex_index)); > + checkGLcall("glUniform1i"); > + } > @@ -1793,7 +1800,8 @@ static void shader_glsl_declare_generic_vertex_attribute(struct wined3d_string_b > > if (e->sysval_semantic == WINED3D_SV_VERTEX_ID) > { > - shader_addline(buffer, "vec4 vs_in%u = vec4(intBitsToFloat(gl_VertexID), 0.0, 0.0, 0.0);\n", > + shader_addline(buffer, "uniform int base_vertex;\n"); > + shader_addline(buffer, "vec4 vs_in%u = vec4(intBitsToFloat(gl_VertexID - base_vertex), 0.0, 0.0, 0.0);\n", > index); > return; > } Does this do the right thing when ARB_draw_elements_base_vertex is not supported?

1 0

Re: [00/23] Fixes to support Overwatch
by Matteo Bruni 14 Nov '16

14 Nov '16

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > This is the first set of patches to support running Overwatch under WINE. > There are additional patches necessary but I am still preparing those for > release. Wow, that's very cool! Thank you for your work! You can expect your patches to be reviewed over the next few days and get either sign-offs or comments about things to improve. Going forward, in general I suggest you to limit your patch series to 5 patches or so (depending on how it makes the most sense), it makes it easier to the reviewers. I'm going to reply to a few patches in your series with the things I noticed at a quick look...

2 1

Re: [PATCH] wined3d: Add Polaris GPUs and their driver.
by Henri Verbeet 14 Nov '16

14 Nov '16

On 13 November 2016 at 23:10, Grazvydas Ignotas <notasas(a)gmail.com> wrote: > - {HW_VENDOR_AMD, CARD_AMD_RADEON_R9, "AMD Radeon R9 290", DRIVER_AMD_R600, 4096}, > + {HW_VENDOR_AMD, CARD_AMD_RADEON_R9, "AMD Radeon R9 290", DRIVER_AMD_RX, 4096}, Is that change intentional?

2 4

Re: [PATCH v2] hidclass.sys: Handle POP and PUSH in device descriptors
by Sebastian Lackner 14 Nov '16

14 Nov '16

On 14.11.2016 15:08, Aric Stewart wrote: > + if (!list_empty(&caps_stack)) > + { > + struct caps_stack *entry, *cursor; > + ERR("%i unpopped device caps on the stack\n", list_count(&caps_stack)); > + LIST_FOR_EACH_ENTRY_SAFE(entry, cursor, &caps_stack, struct caps_stack, entry) > + list_remove(&entry->entry); I assume you also wanted to deallocate the remaining blocks? > + }

2 1

Status of HID in Wine
by Aric Stewart 14 Nov '16

14 Nov '16

Hello everyone, So with this the last few commits I can officially say that WINE now has a fully functioning HID stack for Linux hidraw devices. Thanks to Sebastian Lackner and Alexandre for all the help, reviews and attention paid. It is working and in the state it is now because of their assistance. This means that applications that use hid.dll or direct device access to HID devices will start seeing and being able to interact with devices. This is very exciting but is just the very first step. Of course any application trying to do HID directly would be a wonderful test case, I am sure there are many many things that will need to be refined and added before a real application starts being happy. I do have a very rough, quick, poorly coded test case for HID controllers available to the curious. https://www.codeweavers.com/xfer/aric/HID/hid.exe There are still manual steps required before things will start working. 1) By default no hidraw devices on Linux are given any access to users. So you will need to had rw permission to the /dev/hidraw<X> device that you want to have accessible in Wine. Or if you want to be wild and crazy you can add a new udev rule to add permission to all the devices as they are added. 2) The winebus service is not started by default yet. so you will need to start winebus. (wine net start winebus) This makes things a little complicated. I would recommend testing with a process like this: wine cmd net start winebus <test application> Or you an change the service start for winebus in the registry [System\\CurrentControlSet\\Services\\WineBus] "Start"=dword:00000002 I would love comment, reports, anything as people test various devices and programs. As for more platforms, patches for OS/X have just been submitted so that IOHID devices will work on Mac. Then I have a set of patches adding linux event devices as well. Looking forward, the hope is that then we will be able to build RawInput support on top of HID, and also dinput and xinput support as well. As a fun challenge I have been working to get native dinput from DirectX 9 to work on Wine on top of HID and it is moving forward much better than I expect, with a lot of hacks at present. (native Dinput sees the gamepads and even start trying to read them, but current is not getting reports properly) So tell me how things work! thanks, -aric

2 2

Re: uiautomationcore: add stub dll, with UiaClientsAreListening
by Sebastian Lackner 14 Nov '16

14 Nov '16

On 10.11.2016 08:39, Austin English wrote: > +@ stub UiaClientsAreListening You forgot to add the function to the spec file. Regards, Sebastian

1 0

Re: [PATCH] hidclass.sys: Handle POP and PUSH in device descriptors
by Nikolay Sivov 14 Nov '16

14 Nov '16

> + if (list_count(&caps_stack)) > + { > + struct caps_stack *entry, *cursor; > + ERR("%i unpopped device caps on the stack\n", list_count(&caps_stack)); > + LIST_FOR_EACH_ENTRY_SAFE(entry, cursor, &caps_stack, struct caps_stack, entry) > + list_remove(&entry->entry); > + } > + If you only want to check if there's any elements left, it's faster to do list_empty().

1 0

Re: [PATCH 2/4] include: Add d3d11 error codes.
by Henri Verbeet 14 Nov '16

14 Nov '16

On 14 November 2016 at 00:26, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > +cpp_quote("#define _FACD3D11 (0x87c)") Does that really need a cpp_quote()?

1 0

Re: [PATCH 1/4] ddraw: Use wined3d_device_process_vertices for execute buffers (v2).
by Henri Verbeet 14 Nov '16

14 Nov '16

On 13 November 2016 at 22:12, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > @@ -627,29 +544,58 @@ static HRESULT WINAPI d3d_execute_buffer_Unlock(IDirect3DExecuteBuffer *iface) > static HRESULT WINAPI d3d_execute_buffer_SetExecuteData(IDirect3DExecuteBuffer *iface, D3DEXECUTEDATA *data) ... > + memcpy(&buffer->data, data, data->dwSize); It's not a new issue, but should be blindly accept "dwSize" here?

1 0

Re: [v2] cmd: Command chain fixes
by Hugh McMaster 14 Nov '16

14 Nov '16

On Monday, 14 November 2016 6:42 AM, Flávio J. Saraiva wrote: > I'd like to cancel/recall this patch, which is a partial fix. > > I have detected many issues that can't be properly fixed until the > parsing stage is redone, such as this one, so it's better to only > tackle them after redoing the parsing stage. > > I wanted to keep the tests, but didn't understand how to properly add > them as TODOs. (if it's even possible) Tests in cmd.exe can be marked as todo_wine by using the syntax @todo_wine@ at the start of the relevant line. The wiki page on cmd conformance tests has more information. [1] Thanks for working on this area. Hugh [1] https://wiki.winehq.org/Cmd_Conformance_Tests

1 0

Re: [PATCH] ntdll: Make LdrUnloadDLL() a stub for musl libc.
by Austin English 14 Nov '16

14 Nov '16

On Nov 14, 2016 1:24 AM, "He X" <xw897002528(a)gmail.com> wrote: > > https://bugs.winehq.org/show_bug.cgi?id=41703 Hi Wang, As musl's FAQ points out, you should be checking for the broken behavior explicitly, or fix musl itself: http://wiki.musl-libc.org/wiki/FAQ#Q:_why_is_there_no_MUSL_macro_.3F Thanks for looking into this!

1 0

Re: [PATCH] cabinet: Properly initialize our internal fci structure (bug 36115)(valgrind)
by Huw Davies 14 Nov '16

14 Nov '16

Hi Detlef, It was nice to hear you today :-) > On 13 Nov 2016, at 16:58, Detlef Riekenberg <wine.dev(a)web.de> wrote: > diff --git a/dlls/cabinet/fci.c b/dlls/cabinet/fci.c > index 70d4f3f..5f2e4e7 100644 > --- a/dlls/cabinet/fci.c > +++ b/dlls/cabinet/fci.c > @@ -1036,6 +1036,8 @@ HFCI __cdecl FCICreate( > return NULL; > } > > + /* start with a clean structure */ There’s no need for this comment, the code is obvious > + memset(p_fci_internal, 0, sizeof(FCI_Int)); sizeof(*p_fci_internal) > p_fci_internal->magic = FCI_INT_MAGIC; > p_fci_internal->perf = perf; > p_fci_internal->fileplaced = pfnfiledest; > @@ -1050,26 +1052,9 @@ HFCI __cdecl FCICreate( > p_fci_internal->gettemp = pfnfcigtf; > p_fci_internal->ccab = *pccab; > p_fci_internal->pccab = pccab; > - p_fci_internal->fPrevCab = FALSE; > - p_fci_internal->fNextCab = FALSE; > - p_fci_internal->fSplitFolder = FALSE; > - p_fci_internal->fGetNextCabInVain = FALSE; > p_fci_internal->pv = pv; > - p_fci_internal->cdata_in = 0; > - p_fci_internal->cCompressedBytesInFolder = 0; > - p_fci_internal->cFolders = 0; > - p_fci_internal->cFiles = 0; > - p_fci_internal->cDataBlocks = 0; > p_fci_internal->data.handle = -1; > - p_fci_internal->fNewPrevious = FALSE; > - p_fci_internal->estimatedCabinetSize = 0; > - p_fci_internal->statusFolderTotal = 0; > - p_fci_internal->folders_size = 0; > - p_fci_internal->files_size = 0; > - p_fci_internal->placed_files_size = 0; > - p_fci_internal->pending_data_size = 0; > - p_fci_internal->folders_data_size = 0; > - p_fci_internal->compression = tcompTYPE_NONE; > + /* the memset selected tcompTYPE_NONE */ Again, this comment doesn’t add anything. > p_fci_internal->compress = compress_NONE; > > list_init( &p_fci_internal->folders_list );

1 0

Re: [06/23] ntdll: Map empty page if needed inside system mappings.
by Matteo Bruni 13 Nov '16

13 Nov '16

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > Some anti-debug protections will try to read all of the memory > for a loaded system DLL. This patch will handle the page fault when > it attempts to read the space between sections. > > Signed-off-by: Andrew Wesie <awesie(a)gmail.com> > --- > dlls/ntdll/virtual.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c > index e826fa0..f70e9fc 100644 > --- a/dlls/ntdll/virtual.c > +++ b/dlls/ntdll/virtual.c > @@ -1512,6 +1512,11 @@ NTSTATUS virtual_handle_fault( LPCVOID addr, DWORD err, BOOL on_signal_stack ) > /* ignore fault if page is writable now */ > if (VIRTUAL_GetUnixProt( *vprot ) & PROT_WRITE) ret = STATUS_SUCCESS; > } > + if (err == 0 && (view->protect & VPROT_SYSTEM)) > + { > + wine_anon_mmap(page, page_size, PROT_READ, MAP_FIXED); > + ret = STATUS_SUCCESS; > + } > if (!on_signal_stack && (*vprot & VPROT_GUARD)) > { > VIRTUAL_SetProt( view, page, page_size, *vprot & ~VPROT_GUARD ); This looks like a much nicer solution than my hack attached to the bug. I don't have time right now to check for all the implications of this patch, when / if I can get around to do a proper review I'll sign it off, for what it might be worth.

1 0

Re: [18/23] wined3d: Add support for flat interpolation in shaders.
by Matteo Bruni 13 Nov '16

13 Nov '16

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > Signed-off-by: Andrew Wesie <awesie(a)gmail.com> It looks like you're never declaring shader outputs as flat. Is the potential mismatch with the shader input declarations a problem? BTW, I find it a bit surprising that flat varyings are actually used in SM4+ applications, AFAIK there is no way to mark inputs / outputs as flat in HLSL, which would suggest some kind of manual tinkering with the shader bytecode is required to use them.

3 2

Re: [14/23] wined3d: Increase vertex and fragment sampler limit.
by Matteo Bruni 13 Nov '16

13 Nov '16

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > Signed-off-by: Andrew Wesie <awesie(a)gmail.com> > --- > dlls/wined3d/wined3d_private.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h > index d23b7ef..97f3265 100644 > --- a/dlls/wined3d/wined3d_private.h > +++ b/dlls/wined3d/wined3d_private.h > @@ -176,8 +176,8 @@ static inline enum complex_fixup get_complex_fixup(struct color_fixup_desc fixup > #define MAX_STREAM_OUT 4 > #define MAX_STREAMS 16 > #define MAX_TEXTURES 8 > -#define MAX_FRAGMENT_SAMPLERS 16 > -#define MAX_VERTEX_SAMPLERS 4 > +#define MAX_FRAGMENT_SAMPLERS 32 > +#define MAX_VERTEX_SAMPLERS 32 > #define MAX_COMBINED_SAMPLERS (MAX_FRAGMENT_SAMPLERS + MAX_VERTEX_SAMPLERS) > #define MAX_ACTIVE_LIGHTS 8 > #define MAX_CLIP_DISTANCES WINED3DMAXUSERCLIPPLANES I don't think this is how we want to solve this. See bug 41213 comment 6 and 8 for my point of view on the matter.

1 0

Re: [09/23] wined3d: Fix mapping of SV_VertexID to gl_VertexID.
by Matteo Bruni 13 Nov '16

13 Nov '16

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > SV_VertexID does not have base vertex index applied, but gl_VertexID > does. Fix the vertex ID by subtracting a uniform that is set to the > base vertex index before each draw. > > Signed-off-by: Andrew Wesie <awesie(a)gmail.com> > --- > dlls/wined3d/cs.c | 2 +- > dlls/wined3d/device.c | 6 +++++- > dlls/wined3d/glsl_shader.c | 13 ++++++++++++- > dlls/wined3d/state.c | 7 ++++++- > dlls/wined3d/wined3d_private.h | 1 + > 5 files changed, 25 insertions(+), 4 deletions(-) > > diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c > index 7af0b81..e71c873 100644 > --- a/dlls/wined3d/cs.c > +++ b/dlls/wined3d/cs.c > @@ -481,7 +481,7 @@ void wined3d_cs_emit_draw(struct wined3d_cs *cs, int base_vertex_idx, unsigned i > > op = cs->ops->require_space(cs, sizeof(*op)); > op->opcode = WINED3D_CS_OP_DRAW; > - op->base_vertex_idx = base_vertex_idx; > + op->base_vertex_idx = indexed ? base_vertex_idx : 0; > op->start_idx = start_idx; > op->index_count = index_count; > op->start_instance = start_instance; This should probably be a separate patch. Otherwise it seems good to me.

1 0

Re: [04/23] ntdll: Fix privileged instruction exception code.
by Matteo Bruni 13 Nov '16

13 Nov '16

2016-11-13 12:35 GMT-06:00 Andrew Wesie <awesie(a)gmail.com>: > Signed-off-by: Andrew Wesie <awesie(a)gmail.com> > --- > dlls/ntdll/signal_x86_64.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c > index af16cfd..e2440a6 100644 > --- a/dlls/ntdll/signal_x86_64.c > +++ b/dlls/ntdll/signal_x86_64.c > @@ -2652,7 +2652,6 @@ static void segv_handler( int signal, siginfo_t *siginfo, void *sigcontext ) > WORD err = ERROR_sig(ucontext); > if ((err & 7) == 2 && handle_interrupt( err >> 3, rec, win_context )) break; > rec->ExceptionCode = err ? EXCEPTION_ACCESS_VIOLATION : EXCEPTION_PRIV_INSTRUCTION; > - rec->ExceptionCode = EXCEPTION_ACCESS_VIOLATION; The patch is most likely correct but it would be nice to have a Wine test triggering this codepath.

1 0

Re: [PATCH 5/7] ddraw: Discard the dst vertex buffer if possible.
by Henri Verbeet 13 Nov '16

13 Nov '16

On 30 October 2016 at 15:02, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > Once we have buffer->buffer copies working we can load the source data > into a buffer in SetExecuteData and never have to lock the destination > buffer. In general I think wined3d_device_copy_sub_resource_region() is supposed to work for buffers. Is there something in particular that's broken?

2 1

Re: [PATCH] wininet: Resize buffer when call to InternetCanonicalizeUrlW fails in InternetCrackUrlW
by Marvin 13 Nov '16

13 Nov '16

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

2 1

Re: wineconsole: Correctly handle TrueType font widths
by Akihiro Sagawa 13 Nov '16

13 Nov '16

On Tue, 1 Nov 2016 11:59:30 +0000, Hugh McMaster wrote: > - config->cell_width = tm.tmMaxCharWidth; > + if (tm.tmPitchAndFamily & (TMPF_VECTOR | TMPF_TRUETYPE)) > + config->cell_width = tm.tmAveCharWidth; > + else > + config->cell_width = tm.tmMaxCharWidth; > + Hi Hugh, Why don't you use tmAveCharWidth value for raster (bitmap) fonts? Japanese fixed-pitch bitmap font, such as FixedSys (jvgafix.fon) or Terminal (not avaialble in wine), has a half value of tmMaxCharWidth for tmAveCharWidth. In other words, FixedSys shows tmAveCharWidth = 8, tmMaxCharWidth = 16. This works good in DBCS console window because full-width character (e.g. Kanji) occupies two cells. Thanks, Akihiro Sagawa

3 8

Re: d3drm: Avoid shift overflow when using RGBA_MAKE.
by Henri Verbeet 10 Nov '16

10 Nov '16

On 9 November 2016 at 14:09, Thomas Faber <thomas.faber(a)reactos.org> wrote: > This causes warnings on Arch Linux. > gcc (GCC) 6.2.1 20160830 > > In file included from ../../include/d3drmdef.h:25:0, > from ../../include/d3drmobj.h:25, > from ../../include/d3drm.h:27, > from d3drm_private.h:24, > from frame.c:30: > frame.c: In function ‘d3drm_frame3_SetSceneBackgroundRGB’: > ../../include/d3dtypes.h:56:51: error: result of ‘255 << 24’ requires 33 > bits to represent, but ‘int’ only has 32 bits [-Werror=shift-overflow=] > #define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((a) << 24) | ((r) << 16) | > ((g) << 8) | (b))) > ^ > frame.c:2007:30: note: in expansion of macro ‘RGBA_MAKE’ > frame->scenebackground = RGBA_MAKE((BYTE)(red * 255.0f), > ^~~~~~~~~ > frame.c: In function ‘d3drm_frame_create’: > ../../include/d3dtypes.h:56:51: error: result of ‘255 << 24’ requires 33 > bits to represent, but ‘int’ only has 32 bits [-Werror=shift-overflow=] > #define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((a) << 24) | ((r) << 16) | > ((g) << 8) | (b))) > ^ > frame.c:2952:31: note: in expansion of macro ‘RGBA_MAKE’ > object->scenebackground = RGBA_MAKE(0, 0, 0, 0xff); > ^~~~~~~~~ > cc1: all warnings being treated as errors > I've just sent a different patch (https://source.winehq.org/patches/data/127691), that hopefully also addresses this.

2 1

Re: [PATCH 3/3] dpnet: Implement IDirectPlay8Client Enumhost when called synchronously
by Marvin 10 Nov '16

10 Nov '16

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=26583 Your paranoid android. === w7u (32 bit client) === client.c:247: Test failed: got 0x80004005 client.c:247: Test failed: got 0x00000002 client.c:251: Test failed: got 0x80004005 client.c:251: Test failed: got 0x00000003

1 0

invalid wininet.h on the testbot?
by Donat Enikeev 09 Nov '16

09 Nov '16

Hi Guys, Seems testbot can't build tests depending on <wininet.h> I submitted a patch fixing crypt32/chain test failures on linux, and got errors like this: In file included from ../../../../wine/dlls/crypt32/tests/chain.c:28:0: ../../../../wine/include/wininet.h:1:1: error: stray ‘\35’ in program ../../../../wine/include/wininet.h:1:3: warning: null character(s) ignored [enabled by default] Job: https://testbot.winehq.org/JobDetails.pl?Key=26545&log_101=1#k101 The patch does nothing with <wininet.h>, it contains fresh cs.stanford.edu certificates chain, correspondent adjustments, and some broken() removals To confirm that I submitted dummy patch adding only /***/ in one line, and met same problem (job: https://testbot.winehq.org/JobDetails.pl?Key=26548) Best, Donnie

1 0

Re: [PATCH] dpnet: Implement IDirectPlay8Client EnumHosts
by Bruno Jesus 09 Nov '16

09 Nov '16

On Mon, Nov 7, 2016 at 6:24 AM, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > This patch allows the dpnet examples and various games to find the > correct server running on the local network. (Server is either on > windows or using native directplay). Hi, thanks for the wonderful work. Can you share a few game titles (better with demo available) that can be used to test? > Feedback welcome. (Improvements, howto split the patch into smaller chunks). Some ideas below, this is not really a review as I don't understand some mechanics, it is more like a set of questions that may or may not help you. > Changes to CancelAsyncOperation have be obmitted, as it deserves a patch > of it's own. Currently no application tested seems to care if the operation > isn't cancelled. > > > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/dpnet/client.c | 399 ++++++++++++++++++++++++++++++++++++++++++++- > dlls/dpnet/dpnet_private.h | 25 +++ > dlls/dpnet/dppacket.h | 240 +++++++++++++++++++++++++++ > dlls/dpnet/tests/client.c | 81 ++++++++- > 4 files changed, 738 insertions(+), 7 deletions(-) > create mode 100644 dlls/dpnet/dppacket.h > > diff --git a/dlls/dpnet/client.c b/dlls/dpnet/client.c > index b92d940..1227be6 100644 > --- a/dlls/dpnet/client.c > +++ b/dlls/dpnet/client.c > @@ -29,10 +29,12 @@ > #include "wingdi.h" > #include "winuser.h" > #include "objbase.h" > +#include "ws2tcpip.h" > #include "wine/debug.h" > > #include "dplay8.h" > #include "dpnet_private.h" > +#include "dppacket.h" > > WINE_DEFAULT_DEBUG_CHANNEL(dpnet); > > @@ -92,6 +94,21 @@ static ULONG WINAPI IDirectPlay8ClientImpl_Release(IDirectPlay8Client *iface) > > if (!ref) > { > + struct async_tasks *cursor, *cursor2; > + > + SetEvent(This->closeevent); > + closesocket(This->sock); > + > + LIST_FOR_EACH_ENTRY_SAFE(cursor, cursor2, &This->tasks, struct async_tasks, entry) > + { > + IDirectPlay8Address_Release(cursor->device); > + list_remove(&cursor->entry); > + heap_free(cursor); > + } > + > + CloseHandle(This->datathread); > + CloseHandle(This->closeevent); > + > heap_free(This->username); > heap_free(This->data); > heap_free(This); > @@ -130,6 +147,263 @@ static HRESULT WINAPI IDirectPlay8ClientImpl_EnumServiceProviders(IDirectPlay8Cl > return DPN_OK; > } > > +static SOCKET find_free_socket(void) This function does more than finding a free socket, it also configures it as expected by directplay, what about "setup_socket"? > +{ > + SOCKET sock; > + struct sockaddr_in addr; > + int value; > + int port = 2302; > + ULONG l = 1; Where does this number 2302 comes from? A comment would be useful here, the ULONG variable deserves a better name, what about discarding the "value" variable and using only the ULONG in favor of it? There is a mandatory cast in setsockopt so it wouldn't be a problem. As a style choice I would group the int declarations in the same line. > + sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); > + if(sock == INVALID_SOCKET) > + { > + ERR("Cannot create socket.\n"); We all known this should never happen but since we have the ERR what about adding the WSAGetLastError() to it? > + return sock; > + } > + > + memset(&addr, 0, sizeof(addr)); > + addr.sin_family = AF_INET; > + addr.sin_port = htons(port); > + addr.sin_addr.s_addr = htonl(INADDR_ANY); > + > + value = 1; > + setsockopt(sock, SOL_SOCKET, SO_BROADCAST, (char *)&value, sizeof(value)); > + ioctlsocket(sock, FIONBIO, &l); As commented above what about using the ULONG in both functions? Also, why not do this only after a successful bind? > + > + while(port < 2400) Yet another magic number, just add a comment stating this is the final checked port for directplay (if that is what this means). > + { > + if(!bind(sock, (struct sockaddr*)&addr, sizeof(addr))) > + { > + TRACE("bound to port (%d) successful.\n", port); > + return sock; > + } > + if(WSAGetLastError() == WSAEADDRINUSE) > + addr.sin_port = htons(++port); > + else > + break; > + } > + > + ERR("No free ports available.\n"); The previous TRACE call was not capitalized, both ERR calls have the first word capitalized, is it better to always start in uppercase or lowercase? This applies to the rest of the TRACE calls that are mixing upper/lowercase > + closesocket(sock); > + sock = INVALID_SOCKET; > + > + return sock; Maybe rewrite the socket() call check using a block or goto to use a single return point in the function? > +} > + > +struct dpthread_data > +{ > + SOCKET sock; > + DWORD bufsize; > + > + HANDLE event; > + PFNDPNMESSAGEHANDLER msghandler; > + > + struct list *tasks; > +}; > + > +static DWORD WINAPI data_thread(void *threaddata) > +{ > + struct dpthread_data *tdata = (struct dpthread_data *)threaddata; > + struct sockaddr_in addr; > + int addrlen; > + WORD payloadvalue = 0; > + char *buffer = heap_alloc(tdata->bufsize); Is it work to check the allocation? > + int buflen; > + int received; > + struct async_tasks *cursor; > + struct ENUM_QUERY query; > + > + /* This data doesn't change */ > + query.lead = 0x00; > + query.command = PACKET_ENUM_QUERY; > + query.type = PACKET_QUERY_GUID; > + > + TRACE("data thread start.\n"); > + > + while(TRUE) > + { > + if(list_count(tdata->tasks)) > + { > + LIST_FOR_EACH_ENTRY(cursor, tdata->tasks, struct async_tasks, entry) > + { > + if(cursor->retrycnt && (GetTickCount() - cursor->starttime) >= cursor->retrytime) > + { > + int err; > + > + TRACE("Sending Enum Query Request.\n"); Is it worth to also display the current retrycnt? > + > + query.payload = payloadvalue++; > + query.application = cursor->app_guid; > + > + addr.sin_family = AF_INET; > + addr.sin_port = htons(DPNA_DPNSVR_PORT); > + /* TODO - Support a single host */ > + addr.sin_addr.s_addr = INADDR_BROADCAST; > + > + err = sendto(tdata->sock, (void *)&query, sizeof(query), 0, (struct sockaddr *)&addr, sizeof(struct sockaddr_in)); Double space after "0, ". > + if(err == -1) > + ERR("sendto failed (%d).\n", WSAGetLastError()); > + else > + TRACE("sent (%d).\n", err); This TRACE is not necessary since we already have a trace before and an ERR in case of problems. > + > + if(cursor->retrycnt != INFINITE) > + cursor->retrycnt--; > + > + cursor->starttime = GetTickCount(); > + } > + } > + > + /* Query Enumeration Packets */ > + buflen = tdata->bufsize; > + addrlen = sizeof(addr); > + received = recvfrom(tdata->sock, buffer, buflen, 0, (struct sockaddr *)&addr, &addrlen); > + if(received >= (int)sizeof(struct ENUM_HEADER)) There is no else to catch recvfrom errors, probably intentional since this is an UDP socket, right? > + { > + struct ENUM_HEADER *header = (struct ENUM_HEADER *)buffer; > + > + if(header->lead == 0) > + { > + /* Enum Query */ > + switch(header->command) > + { > + case PACKET_ENUM_QUERY: > + { > + TRACE("Ignoring EnumQuery Packet.\n"); > + break; > + } > + /* Enum response */ > + case PACKET_ENUM_RESPONSE: > + { > + DPNMSG_ENUM_HOSTS_RESPONSE response; > + DPN_APPLICATION_DESC desc; > + struct ENUM_QUERY_RESPONSE *data = (struct ENUM_QUERY_RESPONSE *)header; > + IDirectPlay8Address *sender = NULL; > + char ip[INET6_ADDRSTRLEN] = {0}; Is it possible to be really an IPv6? > + int port; > + HRESULT ret; > + > + TRACE("PACKET_ENUM_RESPONSE.\n"); > + > + if(received <= sizeof(struct ENUM_QUERY_RESPONSE)) > + { > + ERR("May not have received enough data.\n"); > + SetEvent(tdata->event); > + break; Is the <= intentional? In my imagination it would be correct to receive == sizeof(struct). This should only happen due to game bugs, right? > + } > + > + memset(&response, 0, sizeof(DPNMSG_ENUM_HOSTS_RESPONSE)); > + memset(&desc, 0, sizeof(DPN_APPLICATION_DESC)); > + > + response.dwSize = sizeof(DPNMSG_ENUM_HOSTS_RESPONSE); > + > + ret = DPN_OK; > + > + LIST_FOR_EACH_ENTRY(cursor, tdata->tasks, struct async_tasks, entry) This is the very same loop in the beginning of the function, what are these tasks and why do we need to run through all of them while inside the outer loop on the same struct? > + { > + if(IsEqualGUID(&cursor->app_guid, &data->application) || > + IsEqualGUID(&cursor->app_guid, &IID_NULL)) /* all applications */ > + { Just a style preference, why not invert the if condition to reduce the code indentation? > + /* > + * We need to create an Address Object with the Port and IP address. > + * Applications are required to AddRef this object if they want to keep a reference. > + */ > + ret = DPNET_CreateDirectPlay8Address(NULL, NULL, &IID_IDirectPlay8Address, (void**)&sender); > + if(FAILED(ret)) > + break; > + > + inet_ntop(addr.sin_family, &addr.sin_addr, ip, sizeof(ip)); > + port = ntohs(addr.sin_port); > + TRACE("Server IP address %s:%d\n", ip, port); > + > + if(FAILED(IDirectPlay8Address_AddComponent(sender, DPNA_KEY_HOSTNAME, &ip, strlen(ip)+1, DPNA_DATATYPE_STRING_ANSI))) > + { > + ERR("Failed to add DPNA_KEY_HOSTNAME\n"); > + IDirectPlay8Address_Release(sender); > + SetEvent(tdata->event); > + break; If CreateDirectPlay8Address fails you are not calling SetEvent, is that intentional? Because if IDirectPlay8Address_AddComponent fails you call SetEvent. > + } > + if(FAILED(IDirectPlay8Address_AddComponent(sender, DPNA_KEY_PORT, &port, sizeof(DWORD), DPNA_DATATYPE_DWORD))) > + { > + ERR("Failed to add DPNA_KEY_PORT\n"); > + IDirectPlay8Address_Release(sender); > + SetEvent(tdata->event); > + break; > + } > + > + response.pAddressSender = sender; > + response.pApplicationDescription = &desc; > + response.pAddressDevice = cursor->device; > + > + desc.guidInstance = data->instance; > + desc.guidApplication = data->application; > + desc.dwCurrentPlayers = data->current_players; > + desc.dwMaxPlayers = data->max_players; > + > + if(data->session_offset && data->session_offset < received) > + desc.pwszSessionName = (WCHAR*)(((char*)data)+data->session_offset+sizeof(struct ENUM_HEADER)); > + > + if(tdata->msghandler) > + ret = tdata->msghandler(NULL, DPN_MSGID_ENUM_HOSTS_RESPONSE, &response); If all the above was only made if a msghandler is present can you simply ignore all data received when msghandler is not present? > + > + IDirectPlay8Address_Release(sender); > + > + if(cursor->retrycnt != INFINITE) > + cursor->retrycnt = 0; > + > + /* If a userhandler function returns anything other than DPN_OK we need to stop. */ > + if(ret != DPN_OK) > + break; > + } > + } > + > + if(ret != DPN_OK) > + SetEvent(tdata->event); Why can't you SetEvent inside the previous ret != DPN_OK since it is breaking the inner loop anyway? > + > + break; > + } > + default: > + { > + FIXME("Unsupported query command 0x%08x.\n", header->command); > + } > + } > + } > + else > + { > + struct DFRAME_PACKET *dframe = (struct DFRAME_PACKET *)buffer; > + FIXME("DFRAME received - command 0x%08x, control 0x%08x.\n", dframe->command, dframe->control); > + } > + } > + > + LIST_FOR_EACH_ENTRY(cursor, tdata->tasks, struct async_tasks, entry) > + { > + if(!cursor->retrycnt && (GetTickCount() - cursor->starttime) >= cursor->timeout) > + { > + list_remove( &cursor->entry ); > + IDirectPlay8Address_Release(cursor->device); > + if(cursor->aysncmutex) > + { > + TRACE("Removing async event\n"); > + SetEvent(cursor->aysncmutex); > + } > + /* TODO - Send DPN_MSGID_ASYNC_OP_COMPLETE if not ASYNC */ > + heap_free(cursor); > + } > + } > + } > + > + if(WaitForSingleObject(tdata->event, 500) != WAIT_TIMEOUT) > + { > + TRACE("Event Signaled.\n"); > + break; > + } > + } > + > + heap_free(buffer); > + > + return 0; > +} > + > static HRESULT WINAPI IDirectPlay8ClientImpl_EnumHosts(IDirectPlay8Client *iface, > PDPN_APPLICATION_DESC const pApplicationDesc, IDirectPlay8Address * const pAddrHost, > IDirectPlay8Address * const pDeviceInfo, void * const pUserEnumData, > @@ -137,9 +411,111 @@ static HRESULT WINAPI IDirectPlay8ClientImpl_EnumHosts(IDirectPlay8Client *iface > const DWORD dwTimeOut, void * const pvUserContext, DPNHANDLE * const pAsyncHandle, > const DWORD dwFlags) > { > - IDirectPlay8ClientImpl *This = impl_from_IDirectPlay8Client(iface); > - FIXME("(%p):(%p,%p,%x): Stub\n", This, pvUserContext, pAsyncHandle, dwFlags); > - return DPN_OK; > + IDirectPlay8ClientImpl *This = impl_from_IDirectPlay8Client(iface); > + HRESULT hr; > + HANDLE async_mutex = NULL; > + struct async_tasks *task; > + > + TRACE("(%p):(%p,%p,%p,%p,%u,%u,%u,%u,%p,%p,%x)\n", This, pApplicationDesc, pAddrHost, pDeviceInfo, pUserEnumData, > + dwUserEnumDataSize, dwEnumCount, dwRetryInterval, dwTimeOut, pvUserContext, pAsyncHandle, dwFlags); > + > + if(!This->msghandler) > + return DPNERR_UNINITIALIZED; > + > + if(dwFlags & DPNENUMHOSTS_SYNC) > + { > + if(pAsyncHandle) > + return DPNERR_INVALIDPARAM; > + > + FIXME("Flag DPNENUMHOSTS_SYNC currently not supported (%u, %u, %u).\n", dwEnumCount, dwRetryInterval, dwTimeOut); > + } > + > + if(dwUserEnumDataSize > This->spcaps.dwMaxEnumPayloadSize) > + return DPNERR_ENUMQUERYTOOLARGE; > + > + if(This->sock == INVALID_SOCKET) > + { > + This->sock = find_free_socket(); > + if(This->sock == INVALID_SOCKET) > + return DPNERR_USERCANCEL; /* Pretend the user cancalled this operation. */ > + } > + > + if(!This->closeevent) > + { > + This->closeevent = CreateEventA( NULL, TRUE, FALSE, NULL); > + if(!This->closeevent) > + { > + WARN("Failed to create Event\n"); > + closesocket(This->sock); > + This->sock = INVALID_SOCKET; > + return DPNERR_GENERIC; > + } > + } > + > + task = heap_alloc(sizeof(struct async_tasks)); > + if(!task) > + { > + closesocket(This->sock); > + This->sock = INVALID_SOCKET; > + return E_OUTOFMEMORY; Shoud you destroy the closeevent too? > + } > + > + task->handle = PtrToUlong(task); > + task->type = enumhost; > + task->app_guid = pApplicationDesc->guidApplication; > + task->starttime = 0; > + task->retrycnt = !dwEnumCount ? This->spcaps.dwDefaultEnumCount : dwEnumCount; > + task->retrytime = dwRetryInterval == INFINITE ? This->spcaps.dwDefaultEnumRetryInterval : dwRetryInterval; > + task->timeout = dwTimeOut == 0 ? This->spcaps.dwDefaultEnumTimeout : dwTimeOut; !dwEnumCount but dwTimeout == 0, style inconsistency? > + if(dwFlags & DPNENUMHOSTS_SYNC) > + async_mutex = CreateMutexA(NULL, FALSE, NULL); > + > + task->aysncmutex = async_mutex; > + hr = IDirectPlay8Address_Duplicate(pDeviceInfo, &task->device); > + if(FAILED(hr)) > + { > + ERR("Failed to duplicate Device address (0x%08x).\n", hr); > + > + closesocket(This->sock); > + This->sock = INVALID_SOCKET; > + heap_free(task); > + > + return E_OUTOFMEMORY; Free closeevent? Free async_mutex? > + } > + > + list_add_tail(&This->tasks, &task->entry); > + > + if(!This->datathread) > + { > + struct dpthread_data *tdata = heap_alloc(sizeof(struct dpthread_data)); > + if(!tdata) > + { > + closesocket(This->sock); > + This->sock = INVALID_SOCKET; > + > + list_remove( &task->entry ); > + IDirectPlay8Address_Release(task->device); > + heap_free(task); > + > + return E_OUTOFMEMORY; > + } > + > + tdata->sock = This->sock; > + tdata->bufsize = This->spcaps.dwSystemBufferSize; > + tdata->msghandler = This->msghandler; > + tdata->event = This->closeevent; > + tdata->tasks = &This->tasks; > + > + This->datathread = CreateThread(NULL, 0, &data_thread, tdata, 0, 0); > + } > + > + if(async_mutex) > + WaitForSingleObject(async_mutex,INFINITE); > + > + if(pAsyncHandle) > + *pAsyncHandle = task->handle; > + > + return (dwFlags & DPNENUMHOSTS_SYNC) ? DPN_OK : DPNSUCCESS_PENDING; > } > > static HRESULT WINAPI IDirectPlay8ClientImpl_CancelAsyncOperation(IDirectPlay8Client *iface, > @@ -256,8 +632,23 @@ static HRESULT WINAPI IDirectPlay8ClientImpl_GetServerAddress(IDirectPlay8Client > static HRESULT WINAPI IDirectPlay8ClientImpl_Close(IDirectPlay8Client *iface, const DWORD dwFlags) > { > IDirectPlay8ClientImpl *This = impl_from_IDirectPlay8Client(iface); > + struct async_tasks *cursor, *cursor2; > FIXME("(%p):(%x): Stub\n", This, dwFlags); > > + SetEvent(This->closeevent); > + > + LIST_FOR_EACH_ENTRY_SAFE(cursor, cursor2, &This->tasks, struct async_tasks, entry) > + { > + IDirectPlay8Address_Release(cursor->device); > + list_remove(&cursor->entry); > + heap_free(cursor); > + } > + > + CloseHandle(This->datathread); > + This->datathread = NULL; > + CloseHandle(This->closeevent); > + This->closeevent = NULL; > + > This->msghandler = NULL; > > return DPN_OK; > @@ -376,6 +767,8 @@ HRESULT DPNET_CreateDirectPlay8Client(IClassFactory *iface, IUnknown *pUnkOuter, > > client->IDirectPlay8Client_iface.lpVtbl = &DirectPlay8Client_Vtbl; > client->ref = 1; > + client->sock = INVALID_SOCKET; > + list_init(&client->tasks); > > init_dpn_sp_caps(&client->spcaps); > > diff --git a/dlls/dpnet/dpnet_private.h b/dlls/dpnet/dpnet_private.h > index 89e0777..763b1c2 100644 > --- a/dlls/dpnet/dpnet_private.h > +++ b/dlls/dpnet/dpnet_private.h > @@ -41,6 +41,26 @@ typedef struct IDirectPlay8AddressImpl IDirectPlay8AddressImpl; > typedef struct IDirectPlay8LobbiedApplicationImpl IDirectPlay8LobbiedApplicationImpl; > typedef struct IDirectPlay8ThreadPoolImpl IDirectPlay8ThreadPoolImpl; > > +enum async_type { enumhost, connection, sendmsg }; > + > +struct async_tasks > +{ > + DPNHANDLE handle; > + BYTE type; > + > + IDirectPlay8Address *device; > + > + PFNDPNMESSAGEHANDLER msghandler; > + GUID app_guid; > + DWORD starttime; > + DWORD retrycnt; > + DWORD retrytime; > + DWORD timeout; > + HANDLE aysncmutex; > + > + struct list entry; > +}; > + > /* ------------------ */ > /* IDirectPlay8Client */ > /* ------------------ */ > @@ -62,6 +82,11 @@ struct IDirectPlay8ClientImpl > DWORD datasize; > > DPN_SP_CAPS spcaps; > + > + SOCKET sock; > + HANDLE closeevent; > + HANDLE datathread; > + struct list tasks; > }; > > /* ------------------- */ > diff --git a/dlls/dpnet/dppacket.h b/dlls/dpnet/dppacket.h > new file mode 100644 > index 0000000..297640a > --- /dev/null > +++ b/dlls/dpnet/dppacket.h > @@ -0,0 +1,240 @@ > +/* > + * dpnet packet structures > + * > + * Copyright 2016 Alistair Leslie-Hughes > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA > + */ > +#ifndef __WINE_DPNET_PACKET_H > +#define __WINE_DPNET_PACKET_H > + > +#include <pshpack1.h> > + > +struct ENUM_HEADER > +{ > + BYTE lead; > + BYTE command; > + WORD payload; > +}; > + > +struct ENUM_QUERY > +{ > + BYTE lead; > + BYTE command; > + WORD payload; > + BYTE type; > + GUID application; > +}; > + > +struct ENUM_QUERY_RESPONSE > +{ > + BYTE lead; > + BYTE command; > + WORD payload; > + DWORD reply_offset; > + DWORD response_size; > + DWORD desc_size; > + DWORD desc_flags; > + DWORD max_players; > + DWORD current_players; > + DWORD session_offset; > + DWORD session_size; > + DWORD Password_offset; > + DWORD Password_size; > + DWORD reserved_offset; > + DWORD reserved_size; > + DWORD application_offset; > + DWORD application_size; > + GUID instance; > + GUID application; > +}; This file seems to be private, so can we have all struct attributes starting in lowercase? > + > +struct DFRAME_PACKET > +{ > + BYTE command; > + BYTE control; > + BYTE seq; > + BYTE nseq; > +}; > + > +/* 2.2.1.8 DN_ACK_CONNECT_INFO */ > +struct DN_ACK_CONNECT_INFO > +{ > + DWORD type; > +}; > + > +struct DFRAME_PACKET_KEEPALIVE > +{ > + BYTE command; > + BYTE control; > + BYTE seq; > + BYTE nseq; > + DWORD session; > +}; > + > +struct DN_MESSAGE_CONNECT > +{ > + BYTE command; > + BYTE opcode; > + BYTE msgid; > + BYTE rspid; > + DWORD protocol; > + DWORD session; > + DWORD timestamp; > +}; > + > + > +struct SACK_PACKET > +{ > + BYTE command; > + BYTE opcode; > + BYTE flags; > + BYTE retry; > + BYTE nseq; > + BYTE nrcv; > + WORD padding; > + DWORD timestamp; > +}; > + > +/* 2.2.1.1 DN_INTERNAL_MESSAGE_PLAYER_CONNECT_INFO */ > +struct DN_INTERNAL_MESSAGE_PLAYER_CONNECT_INFO > +{ > + DWORD type; > + DWORD flags; > + DWORD version; > + DWORD name_offset; > + DWORD name_size; > + DWORD data_offset; > + DWORD data_size; > + DWORD pwd_offset; > + DWORD pwd_size; > + DWORD connect_offset; > + DWORD connect_size; > + DWORD url_offset; > + DWORD url_size; > + GUID instance; > + GUID application; > +}; > + > +struct DN_INTERNAL_MESSAGE_PLAYER_CONNECT_INFO_EX > +{ > + DWORD type; > + DWORD flags; > + DWORD version; > + DWORD name_offset; > + DWORD name_size; > + DWORD data_offset; > + DWORD data_size; > + DWORD pwd_offset; > + DWORD pwd_size; > + DWORD connect_offset; > + DWORD connect_size; > + DWORD url_offset; > + DWORD url_size; > + GUID instance; > + GUID application; > + DWORD alt_address_offset; > + DWORD alt_address_size; > +}; > + > +/* 2.2.1.4 DN_SEND_CONNECT_INFO */ > +struct DN_SEND_CONNECT_INFO > +{ > + DWORD type; /* DN_MSG_INTERNAL_SEND_CONNECT_INFO */ > + DWORD reply_offset; > + DWORD reply_size; > + DWORD size; > + DWORD flags; > + DWORD max_players; > + DWORD cur_players; > + DWORD session_offset; > + DWORD session_size; > + DWORD password_offset; > + DWORD password_size; > + DWORD reserved_offset; > + DWORD reserved_size; > + DWORD appdata_offset; > + DWORD appdata_size; > + GUID instance; > + GUID application; > + DWORD dpnid; > + DWORD version; > + DWORD ver_not_used; > + DWORD entry_count; > + DWORD membership_count; > +}; > + > +#define PACKET_ENUM_QUERY 0x02 > +#define PACKET_ENUM_RESPONSE 0x03 > + > +#define PACKET_QUERY_GUID 0x01 > +#define PACKET_QUERY_NO_GUID 0x02 > + > +#define APPLICATIONDESCSIZE 0x00000050 > + > +#define PACKET_COMMAND_DATA 0x01 > +#define PACKET_COMMAND_RELIABLE 0x02 > +#define PACKET_COMMAND_SEQUENTIAL 0x04 > +#define PACKET_COMMAND_POLL 0x08 > +#define PACKET_COMMAND_NEW_MSG 0x10 > +#define PACKET_COMMAND_END_MSG 0x20 > +#define PACKET_COMMAND_USER_1 0x40 > +#define PACKET_COMMAND_USER_2 0x80 > +#define PACKET_COMMAND_CFRAME PACKET_COMMAND_USER_2 > + > +#define PACKET_CONTROL_RETRY 0x01 > +#define PACKET_CONTROL_KEEPALIVE_OR_CORRELATE 0x02 > +#define PACKET_CONTROL_COALESCE 0x04 > +#define PACKET_CONTROL_END_STREAM 0x08 > +#define PACKET_CONTROL_SACK1 0x10 > +#define PACKET_CONTROL_SACK2 0x20 > +#define PACKET_CONTROL_SEND1 0x40 > +#define PACKET_CONTROL_SEND2 0x80 > + > +#define PROTOCOL_VERSION 0x00010006 > + > +#define FRAME_EXOPCODE_CONNECT 0x01 > +#define FRAME_EXOPCODE_CONNECTED 0x02 > +#define FRAME_EXOPCODE_CONNECTED_SIGNED 0x03 > +#define FRAME_EXOPCODE_HARD_DISCONNECT 0x04 > +#define FRAME_EXOPCODE_SACK 0x06 > + > +#define PACKET_SIGNING_FAST 0x00000001 > +#define PACKET_SIGNING_FULL 0x00000002 > + > +#define SACK_FLAGS_RESPONSE 0x01 > +#define SACK_FLAGS_SACK_MASK1 0x02 > +#define SACK_FLAGS_SACK_MASK2 0x04 > +#define SACK_FLAGS_SEND_MASK1 0x08 > +#define SACK_FLAGS_SEND_MASK2 0x10 > + > +#define DN_MSG_INTERNAL_PLAYER_CONNECT_INFO 0x000000C1 > +#define DN_MSG_INTERNAL_SEND_CONNECT_INFO 0x000000C2 > +#define DN_MSG_INTERNAL_ACK_CONNECT_INFO 0x000000C3 > + > +#define DP_OBECT_TYPE_CLIENT 0x00000002 > +#define DP_OBECT_TYPE_PEER 0x00000004 > + > +#define DP_OBECT_NET_VER_8_0 0x00000001 > +#define DP_OBECT_NET_VER_8_1 0x00000002 > +#define DP_OBECT_NET_VER_POCKETPC 0x00000003 > +#define DP_OBECT_NET_VER_W2K3 0x00000005 > +#define DP_OBECT_NET_VER_8_2 0x00000006 > +#define DP_OBECT_NET_VER_9_0 0x00000007 > +#define DP_OBECT_NET_VER_9_0A 0x00000008 This file seems to be adding way more stuff than you are using currently, is it better to add as things are developed? > +#include "poppack.h" > + > +#endif > diff --git a/dlls/dpnet/tests/client.c b/dlls/dpnet/tests/client.c > index 4753d74..608f0e5 100644 > --- a/dlls/dpnet/tests/client.c > +++ b/dlls/dpnet/tests/client.c > @@ -29,9 +29,29 @@ static IDirectPlay8Client* client = NULL; > static IDirectPlay8LobbiedApplication* lobbied = NULL; > static const GUID appguid = { 0xcd0c3d4b, 0xe15e, 0x4cf2, { 0x9e, 0xa8, 0x6e, 0x1d, 0x65, 0x48, 0xc5, 0xa5 } }; > > +static HRESULT lastAsyncCode = E_FAIL; > +static DPNHANDLE lastAsyncHandle = 0; > + > +#define CHECK_LAST_ASYNC_OP(hr, handle) \ > + ok(lastAsyncCode == hr, "got 0x%08x\n", lastAsyncCode); \ > + ok(lastAsyncHandle == handle, "got 0x%08x\n", handle); \ > + lastAsyncCode = E_FAIL; lastAsyncHandle = 0; > + > static HRESULT WINAPI DirectPlayMessageHandler(PVOID context, DWORD message_id, PVOID buffer) > { > - trace("DirectPlayMessageHandler: 0x%08x\n", message_id); > + switch(message_id) > + { > + case DPN_MSGID_ASYNC_OP_COMPLETE: > + { > + PDPNMSG_ASYNC_OP_COMPLETE async_msg = (PDPNMSG_ASYNC_OP_COMPLETE)buffer; > + lastAsyncCode = async_msg->hResultCode; > + lastAsyncHandle = async_msg->hAsyncOp; > + break; > + } > + default: > + trace("DirectPlayMessageHandler: 0x%08x\n", message_id); > + > + } > return S_OK; > } > > @@ -144,16 +164,22 @@ static void test_enum_service_providers(void) > static void test_enum_hosts(void) > { > HRESULT hr; > + IDirectPlay8Client *client2; > IDirectPlay8Address *host = NULL; > IDirectPlay8Address *local = NULL; > DPN_APPLICATION_DESC appdesc; > - DPNHANDLE async = 0; > + DPNHANDLE async = 0, async2 = 0; > static const WCHAR localhost[] = {'1','2','7','.','0','.','0','.','1',0}; > + DPN_SP_CAPS caps; > + char *data; > > memset( &appdesc, 0, sizeof(DPN_APPLICATION_DESC) ); > appdesc.dwSize = sizeof( DPN_APPLICATION_DESC ); > appdesc.guidApplication = appguid; > > + hr = CoCreateInstance(&CLSID_DirectPlay8Client, NULL, CLSCTX_INPROC_SERVER, &IID_IDirectPlay8Client, (void **)&client2); > + ok(hr == S_OK, "CoCreateInstance failed with 0x%x\n", hr); > + > hr = CoCreateInstance( &CLSID_DirectPlay8Address, NULL, CLSCTX_ALL, &IID_IDirectPlay8Address, (LPVOID*)&local); > ok(hr == S_OK, "IDirectPlay8Address failed with 0x%08x\n", hr); > > @@ -170,16 +196,63 @@ static void test_enum_hosts(void) > DPNA_DATATYPE_STRING); > ok(hr == S_OK, "IDirectPlay8Address failed with 0x%08x\n", hr); > > + caps.dwSize = sizeof(DPN_SP_CAPS); > + > + hr = IDirectPlay8Client_GetSPCaps(client, &CLSID_DP8SP_TCPIP, &caps, 0); > + ok(hr == DPN_OK, "got %x\n", hr); > + data = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, caps.dwMaxEnumPayloadSize + 1); > + > + hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, NULL, 0, 2, 1000, 1000, NULL, &async, DPNENUMHOSTS_SYNC); > + ok(hr == DPNERR_INVALIDPARAM, "got 0x%08x\n", hr); > + > + hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, data, caps.dwMaxEnumPayloadSize + 1, > + INFINITE, 0, INFINITE, NULL, &async, DPNENUMHOSTS_SYNC); > + ok(hr == DPNERR_INVALIDPARAM, "got 0x%08x\n", hr); > + > + async = 0; > + hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, data, caps.dwMaxEnumPayloadSize + 1, INFINITE, 0, > + INFINITE, NULL, &async, 0); > + ok(hr == DPNERR_ENUMQUERYTOOLARGE, "got 0x%08x\n", hr); > + ok(!async, "Handle returned\n"); > + > + async = 0; > + hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, data, caps.dwMaxEnumPayloadSize, INFINITE, 0, INFINITE, > + NULL, &async, 0); > + ok(hr == DPNSUCCESS_PENDING, "got 0x%08x\n", hr); > + ok(async, "No Handle returned\n"); > + > + /* This CancelAsyncOperation doesn't generate a DPN_MSGID_ASYNC_OP_COMPLETE */ > + hr = IDirectPlay8Client_CancelAsyncOperation(client, async, 0); > + ok(hr == S_OK, "got 0x%08x\n", hr); > + HeapFree(GetProcessHeap(), 0, data); > + > + /* No Initialize has been called on client2. */ > + hr = IDirectPlay8Client_EnumHosts(client2, &appdesc, host, local, NULL, 0, INFINITE, 0, INFINITE, NULL, &async, 0); > + ok(hr == DPNERR_UNINITIALIZED, "IDirectPlay8Client_EnumHosts failed with 0x%08x\n", hr); > + > /* Since we are running asynchronously, EnumHosts returns DPNSUCCESS_PENDING. */ > hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, NULL, 0, INFINITE, 0, INFINITE, NULL, &async, 0); > - todo_wine ok(hr == DPNSUCCESS_PENDING, "IDirectPlay8Client_EnumServiceProviders failed with 0x%08x\n", hr); > - todo_wine ok(async, "No Handle returned\n"); > + ok(hr == DPNSUCCESS_PENDING, "IDirectPlay8Client_EnumHosts failed with 0x%08x\n", hr); > + ok(async, "No Handle returned\n"); > + > + hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, NULL, 0, INFINITE, 0, INFINITE, NULL, &async2, 0); > + ok(hr == DPNSUCCESS_PENDING, "IDirectPlay8Client_EnumHosts failed with 0x%08x\n", hr); > + ok(async2, "No Handle returned\n"); > + ok(async2 != async, "Same handle returned.\n"); > + > + Sleep(500); > > hr = IDirectPlay8Client_CancelAsyncOperation(client, async, 0); > ok(hr == S_OK, "IDirectPlay8Client_CancelAsyncOperation failed with 0x%08x\n", hr); > + todo_wine { CHECK_LAST_ASYNC_OP(DPNERR_USERCANCEL, async); } > + > + hr = IDirectPlay8Client_CancelAsyncOperation(client, async2, 0); > + ok(hr == S_OK, "IDirectPlay8Client_CancelAsyncOperation failed with 0x%08x\n", hr); > + todo_wine { CHECK_LAST_ASYNC_OP(DPNERR_USERCANCEL, async2); } > > IDirectPlay8Address_Release(local); > IDirectPlay8Address_Release(host); > + IDirectPlay8Client_Release(client2); > } > > static void test_get_sp_caps(void) > -- > 1.9.1 > > >

1 0

Re: [PATCH] winex11.drv: import X11's "text/html" as "HTML Format" (try 2)
by Alexandre Julliard 08 Nov '16

08 Nov '16

Damjan Jovanovic <damjan.jov(a)gmail.com> writes: > + hClipData = GlobalAlloc(GMEM_MOVEABLE | GMEM_DDESHARE, clipDataSize); > + if (hClipData) > + { > + char *htmlFormat = GlobalLock(hClipData); > + snprintf(htmlFormat, clipDataSize, "%s%s%s%s", description, startFragment, textHtml, endFragment); > + TRACE("incoming \"HTML Format\" with these contents:\n%s\n", htmlFormat); > + GlobalUnlock(hClipData); > + } This should be GMEM_FIXED now. Please check how it's done in other functions. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 2/2] user32: Check for PeekMessage failure in MENU_SuspendPopup.
by Alexandre Julliard 08 Nov '16

08 Nov '16

Alex Henrie <alexhenrie24(a)gmail.com> writes: > Fixes https://bugs.winehq.org/show_bug.cgi?id=41508 > > The entire MENU_SuspendPopup function is just an optimization to try to > improve performance if the left or right arrow key is held down. If the > next message is not recognized, it should be processed and removed > normally, without optimization. Probably what you want is to add handling of WM_SYSKEYDOWN instead. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH] d2d1/tests: Some tests for GetRect() and GetRoundedRect()
by Henri Verbeet 08 Nov '16

08 Nov '16

On 8 November 2016 at 15:05, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/d2d1/tests/d2d1.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 88 insertions(+), 1 deletion(-) > This could probably be split into two patches, although I don't feel too strongly about it. > + set_rect(&rect, 0.0f, 0.0f, 0.0f, 0.0f); > + hr = ID2D1Factory_CreateRectangleGeometry(factory, &rect, &geometry); > + ok(SUCCEEDED(hr), "Failed to create geometry, hr %#x.\n", hr); > + ID2D1RectangleGeometry_GetRect(geometry, &rect2); > + ok(!memcmp(&rect, &rect2, sizeof(rect)), "Got wrong rectangle.\n"); We tend to go with "unexpected" instead of "wrong", although that's just fwiw. I do think it would be helpful to print the contents of "rect2" here when the test fails.

1 0

Re: [PATCH 4/5] wined3d: Use unsynchronized buffer updates only if there has been at least one buffer map.
by Henri Verbeet 08 Nov '16

08 Nov '16

On 4 November 2016 at 13:43, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Fixes bc019fcbbd7f0014280a292bc58ef618cfbe34c7. We cannot use > unsynchronized updates when the application haven't called map at all. > Mostly out of curiosity, how is this triggered in practice? Also, would it make sense to track synchronised/unsynchronised in buffer_invalidate_bo_range() instead?

2 2

Re: [PATCH 3/5] d3d10core/tests: Fix test failures in test_render_target_views().
by Henri Verbeet 08 Nov '16

08 Nov '16

On 8 November 2016 at 12:11, Józef Kucia <jkucia(a)codeweavers.com> wrote: > The initial texture data is not necessarily initialized to 0. > That's different from at least d3d9. It probably also implies we can skip clearing the initial texture contents for d3d10+.

1 0

Re: [PATCH 5/5] d3d8/tests: Accept broken behavior for lost device tests.
by Marvin 08 Nov '16

08 Nov '16

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=26524 Your paranoid android. === w8 (32 bit device) === device.c:2794: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2799: Test failed: The focus window is active. device.c:2807: Test failed: Got unexpected hr 0x88760869. device.c:2811: Test failed: Got unexpect screen size 800x600. device.c:2826: Test failed: Expected message 0x47 for window 0, but didn't receive it, i=1. device.c:2835: Test failed: Got unexpect screen size 800x600. device.c:2848: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2856: Test failed: Got unexpect width 640. device.c:2857: Test failed: Got unexpect height 480. === w864 (32 bit device) === device.c:2794: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2799: Test failed: The focus window is active. device.c:2807: Test failed: Got unexpected hr 0x88760869. device.c:2811: Test failed: Got unexpect screen size 800x600. device.c:2826: Test failed: Expected message 0x47 for window 0, but didn't receive it, i=1. device.c:2835: Test failed: Got unexpect screen size 800x600. device.c:2848: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2856: Test failed: Got unexpect width 640. device.c:2857: Test failed: Got unexpect height 480. === w1064 (32 bit device) === device.c:2794: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2799: Test failed: The focus window is active. device.c:2807: Test failed: Got unexpected hr 0x88760869. device.c:2811: Test failed: Got unexpect screen size 800x600. device.c:2826: Test failed: Expected message 0x47 for window 0, but didn't receive it, i=1. device.c:2835: Test failed: Got unexpect screen size 800x600. device.c:2848: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2856: Test failed: Got unexpect width 640. device.c:2857: Test failed: Got unexpect height 480.

1 0

Re: [v2 PATCH] user32/msgbox: Support WM_COPY Message
by Nikolay Sivov 08 Nov '16

08 Nov '16

On 08.11.2016 13:44, Alistair Leslie-Hughes wrote: > Fixes: https://bugs.winehq.org/show_bug.cgi?id=17205 > > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/user32/msgbox.c | 86 +++++++++++++++++++-- > dlls/user32/tests/dialog.c | 187 ++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 267 insertions(+), 6 deletions(-) > > diff --git a/dlls/user32/msgbox.c b/dlls/user32/msgbox.c > index 2ba98c9..4735b8a 100644 > --- a/dlls/user32/msgbox.c > +++ b/dlls/user32/msgbox.c > @@ -44,6 +44,11 @@ struct ThreadWindows > HWND *handles; > }; > > +/* Index the order the buttons need to appear to an ID* constant */ > +static const int buttonOrder[10] = { IDYES, IDNO, IDOK, IDABORT, IDRETRY, > + IDCANCEL, IDIGNORE, IDTRYAGAIN, > + IDCONTINUE, IDHELP }; > + > static BOOL CALLBACK MSGBOX_EnumProc(HWND hwnd, LPARAM lParam) > { > struct ThreadWindows *threadWindows = (struct ThreadWindows *)lParam; > @@ -77,11 +82,6 @@ static void MSGBOX_OnInit(HWND hwnd, LPMSGBOXPARAMSW lpmb) > WCHAR *buffer = NULL; > const WCHAR *ptr; > > - /* Index the order the buttons need to appear to an ID* constant */ > - static const int buttonOrder[10] = { IDYES, IDNO, IDOK, IDABORT, IDRETRY, > - IDCANCEL, IDIGNORE, IDTRYAGAIN, > - IDCONTINUE, IDHELP }; > - > nclm.cbSize = sizeof(nclm); > SystemParametersInfoW (SPI_GETNONCLIENTMETRICS, 0, &nclm, 0); > > @@ -319,6 +319,77 @@ static void MSGBOX_OnInit(HWND hwnd, LPMSGBOXPARAMSW lpmb) > HeapFree( GetProcessHeap(), 0, buffer ); > } > > +static void MSGBOX_CopyToClipboard( HWND hwnd ) > +{ > + int i; > + static const WCHAR line[] = {'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-', > + '-','-','-','-','-','-','-','-','\r','\n', 0}; > + static const WCHAR crlf[] = {'\r','\n', 0}; > + static const WCHAR spaces[] = {' ',' ',' ', 0}; > + int lenTitle = GetWindowTextLengthW(hwnd) + 1; > + int lenMsg = GetWindowTextLengthW(GetDlgItem(hwnd, MSGBOX_IDTEXT)) + 1; > + > + /* > + --------------------------- > + Dialog Title > + --------------------------- > + Dialog Message > + --------------------------- > + Button(s) Text. OK > + --------------------------- > + */ > + int len = ((sizeof(crlf) * 3) + (sizeof(line) * 4) + lenTitle + lenMsg) * sizeof(WCHAR); This will allocate more than you need. > + WCHAR *text = HeapAlloc( GetProcessHeap(), 0, len); Maybe return right here if allocation failed? Also please use consistent formatting regarding spaces. > + if(text) > + { > + lstrcpyW(text, line); > + if (GetWindowTextW(hwnd, text + lstrlenW(text), lenTitle)) > + { > + HGLOBAL hMem; > + WCHAR *data; > + > + lstrcatW(text, crlf); > + lstrcatW(text, line); > + GetWindowTextW(GetDlgItem(hwnd, MSGBOX_IDTEXT), text + lstrlenW(text), lenMsg); There is GetDlgItemText() for that. > + lstrcatW(text, crlf); > + lstrcatW(text, line); > + > + for (i = 0; i < (sizeof(buttonOrder) / sizeof(buttonOrder[0])); i++) > + { > + HWND hItem = GetDlgItem(hwnd, buttonOrder[i]); > + if (GetWindowLongW(hItem, GWL_STYLE) & WS_VISIBLE) > + { Does IsWindowVisible() work here? > + WCHAR buffer[1024] = {0}; This looks excessive for a button text string. > > case WM_COMMAND: > switch (LOWORD(wParam)) > diff --git a/dlls/user32/tests/dialog.c b/dlls/user32/tests/dialog.c > index a37c678..0bb7dca 100644 > --- a/dlls/user32/tests/dialog.c > +++ b/dlls/user32/tests/dialog.c > @@ -40,6 +40,7 @@ > #include "winbase.h" > #include "wingdi.h" > #include "winuser.h" > +#include "winnls.h" > > #define MAXHWNDS 1024 > static HWND hwnd [MAXHWNDS]; > @@ -1392,6 +1393,187 @@ static void test_MessageBoxFontTest(void) > DestroyWindow(hDlg); > } > > +static const char msgbox_title[] = "%5!z9ZXw*ia;57n/FGl.bCH,Su\"mfKN;foCqAU\'j6AmoJgAc_D:Z0A\'E6PF_O/w"; What's the idea behind such title? If you're trying to create a unique window title, I don't think it has to be that unique. > +static WCHAR expectedOK[] = > +{ > +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n', > +'%','5','!','z','9','Z','X','w','*','i','a',';','5','7','n','/','F','G','l','.','b','C','H',',','S','u','"','m','f', > +'K','N',';','f','o','C','q','A','U','\'','j','6','A','m','o','J','g','A','c','_','D',':','Z','0','A','\'','E','6','P', > +'F','_','O','/','w','\r','\n', > +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n', > +'M','e','s','s','a','g','e','\r','\n', > +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n', > +'O','K',' ',' ',' ','\r','\n', > +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n', 0 > +}; Instead of expected strings you can as well duplicate your implementation logic and generate expected result. This will help testing with localized dialogs. > + > +DWORD WINAPI WorkerThread(void *param) Please please use more specific name. > + { > + succeeded = lstrcmpW(expected, text) == 0; > + if(!succeeded) > + { > + ok(0, "%s\n", wine_dbgstr_w(text)); > + ok(0, "%s\n", wine_dbgstr_w(expected)); > + } > + } Could this simply be ok(!lstrcmpW(), "..."); ? > + > +static void test_MessageBox_WM_COPY_Test(void) > +{ > + DWORD tid = 0; > + > + non_english = (PRIMARYLANGID(GetUserDefaultLangID()) != LANG_ENGLISH); > + trace("non_english %d\n", non_english); > + > + CreateThread(NULL, 0, WorkerThread, &expectedOK, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_OK); > + > + CreateThread(NULL, 0, WorkerThread, &expectedOkCancel, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_OKCANCEL); > + > + CreateThread(NULL, 0, WorkerThread, &expectedAbortRetryIgnore, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_ABORTRETRYIGNORE); > + > + CreateThread(NULL, 0, WorkerThread, &expectedYesNo, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_YESNO); > + > + CreateThread(NULL, 0, WorkerThread, &expectedYesNoCancel, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_YESNOCANCEL); > + > + CreateThread(NULL, 0, WorkerThread, &expectedRetryCancel, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_RETRYCANCEL); > + > + CreateThread(NULL, 0, WorkerThread, &expectedCancelTryContinue, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_CANCELTRYCONTINUE); > +} This could be a loop over possible button values.

1 0

Re: [PATCH] netapi32/test: Add trace to help narrow down failing test.
by Hans Leidekker 08 Nov '16

08 Nov '16

On Tue, 2016-11-08 at 10:07 +0000, Alistair Leslie-Hughes wrote: > Based off a test run with extra traces > https://testbot.winehq.org/JobDetails.pl?Key=26512&log_201=1#k201 > the intermittently failing test is in test_DavGetUNCFromHTTPPath. The failure is a timeout, so you're likely to find a random test. I think we should concentrate on the earlier tests and figure out where all that time is spent.

2 1

Re: [PATCH] user32/msgbox: Support WM_COPY Message
by Dmitry Timoshkov 08 Nov '16

08 Nov '16

Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > +static void MSGBOX_CopyToClipbaord( HWND hwnd ) A typo in the name. > + static const WCHAR carriage[] = {'\r','\n', 0}; A usual name would be crlf[W]. -- Dmitry.

1 0

Re: [PATCH] netapi32/test: Add trace to help narrow down failing test.
by Nikolay Sivov 08 Nov '16

08 Nov '16

On 08.11.2016 13:07, Alistair Leslie-Hughes wrote: > Based off a test run with extra traces > https://testbot.winehq.org/JobDetails.pl?Key=26512&log_201=1#k201 > the intermittently failing test is in test_DavGetUNCFromHTTPPath. > > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/netapi32/tests/access.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/dlls/netapi32/tests/access.c b/dlls/netapi32/tests/access.c > index f095d77..0fab0de 100644 > --- a/dlls/netapi32/tests/access.c > +++ b/dlls/netapi32/tests/access.c > @@ -688,6 +688,7 @@ static void test_DavGetUNCFromHTTPPath(void) > for (i = 0; i < sizeof(tests)/sizeof(tests[0]); i++) > { > buf[0] = 0; > + trace("test: %d\n", i); > size = tests[i].size; > ret = pDavGetUNCFromHTTPPath( tests[i].path, buf, &size ); > ok( ret == tests[i].ret, "%u: expected %u got %u\n", i, tests[i].ret, ret ); ok() lines already print failed test index, and in case of a crash manual test as you is enough.

1 0

Re: [PATCH] vbscript: Coerce datatype in StrComp
by Marvin 08 Nov '16

08 Nov '16

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=26507 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

1 0

Constructive podium thumping at Wineconf?
by Jeremy White 08 Nov '16

08 Nov '16

Hey all, As is tradition, on Saturday I plan to review the winehq test regime, and pound my fist on the table until people commit to fixing tests. We've spent the past few years managing the transition to qemu. I've been using this page to triage that progress: https://www.winehq.org/~jwhite/latest.html That reflects issues with our tests on Windows on qemu, excluding Windows 8 and Windows 10 (although it looks like I could perhaps add Windows 8 back into the mix, as the bleeding does not seem that bad any longer). We've stalled out at about 13 issues; we don't seem to be able to get below that. (There are 8 'old' issues, and 5 new ones, in case anyone wants to go look, including ones from ddraw, kernel32, netapi32, and user32). Prior to the qemu drive, the original goal was to get at least one Linux machine known to run the tests cleanly. The further hope would be that if we had a known set of 'good' test machines, then we could require that every patch succeed when run on those machines. We've made sporadic progress on that, but we did purchase a set of hardware with the intent of making that hardware run those tests cleanly. I see that we've driven those down to about 20 per system. Those machines are 'portable', for generous definitions of portable. But we could bring one or both them to Wineconf and work directly on them if we liked. Would that be useful? And, more broadly, is there some other way to consider this that would be more useful? Is there useful prep we can do that will give way to more constructive sessions at Wineconf? Cheers, Jeremy

2 1

Re: [PATCH resend 5/7] winebus.sys: reports for iohid
by Sebastian Lackner 07 Nov '16

07 Nov '16

On 04.11.2016 13:46, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_iohid.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > v2-0005-winebus.sys-reports-for-iohid.txt > > > diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c > index e1a97ef..7208117 100644 > --- a/dlls/winebus.sys/bus_iohid.c > +++ b/dlls/winebus.sys/bus_iohid.c > @@ -119,6 +119,13 @@ static DWORD CFNumberToDWORD(CFNumberRef num) > return dwNum; > } > > +static void handle_IOHIDDeviceIOHIDReportCallback(void * inContext, > + IOReturn inResult, void * inSender, IOHIDReportType inType, > + uint32_t inReportID, uint8_t * inReport, CFIndex inReportLength) I would suggest to use parameter names without "in" prefix. Also, there usually shouldn't be any space between "*" and the following variable name. > +{ It might be better to store the DEVICE_OBJECT in a local variable before passing it to the next function, to ensure the types are checked correctly. Otherwise it remains unnoticed when the signature of process_hid_report changes in the future. > + process_hid_report(inContext, inReport, inReportLength); > +} > + > static int compare_platform_device(DEVICE_OBJECT *device, void *platform_dev) > { > ULONG_PTR dev1 = (ULONG_PTR)*(IOHIDDeviceRef*)get_platform_private(device); > @@ -179,7 +186,17 @@ static NTSTATUS get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DW > > static NTSTATUS begin_report_processing(DEVICE_OBJECT *device) > { > - return STATUS_NOT_IMPLEMENTED; > + DWORD length; > + uint8_t *buffer; > + IOHIDDeviceRef dev = *(IOHIDDeviceRef*)get_platform_private(device); > + CFNumberRef num; To be compatible with the rest of the code, this should only start processing once. You might have to store the state of report processing in the platform struct. > + > + num = IOHIDDeviceGetProperty(dev, CFSTR(kIOHIDMaxInputReportSizeKey)); > + length = CFNumberToDWORD(num); > + buffer = HeapAlloc(GetProcessHeap(), 0, length); You might want to do some error checking here. > + > + IOHIDDeviceRegisterInputReportCallback(dev, buffer, length, handle_IOHIDDeviceIOHIDReportCallback, device); > + return STATUS_SUCCESS; > } > > static NTSTATUS set_output_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > @@ -244,6 +261,9 @@ static void handle_RemovalCallback(void *inContext, IOReturn inResult, void *inS > { > DEVICE_OBJECT *device; > TRACE("OS/X IOHID Device Removed %p\n", inIOHIDDeviceRef); > + IOHIDDeviceRegisterInputReportCallback(inIOHIDDeviceRef, NULL, 0, NULL, NULL); > + /* Note: Yes, we leak the buffer. But according to research there is no > + safe way to deallocate that buffer. */ Is it a problem when IOHIDDeviceRegisterInputReportCallback is called, although report processing was never started? And why isn't it safe to deallocate the buffer here? I don't think you will get additional reports when the device has been removed. > device = bus_find_hid_device(&iohid_vtbl, inIOHIDDeviceRef); > if (device) > { > > >

2 1

Re: [PATCH v3 1/7] winebus.sys: Add OS/X IOHID bus
by Sebastian Lackner 07 Nov '16

07 Nov '16

On 04.11.2016 13:45, Aric Stewart wrote: > v2: remove Unload code as it will never be triggered > v3: Suggestions from Ken Thomases > Merge Implement adding IOHID devices into this patch > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/Makefile.in | 3 +- > dlls/winebus.sys/bus.h | 1 + > dlls/winebus.sys/bus_iohid.c | 255 +++++++++++++++++++++++++++++++++++++++++++ > dlls/winebus.sys/main.c | 3 + > 4 files changed, 261 insertions(+), 1 deletion(-) > create mode 100644 dlls/winebus.sys/bus_iohid.c > > > > v2-0001-winebus.sys-Add-OS-X-IOHID-bus.txt > > > diff --git a/dlls/winebus.sys/Makefile.in b/dlls/winebus.sys/Makefile.in > index 9d3a0c2..ac02533 100644 > --- a/dlls/winebus.sys/Makefile.in > +++ b/dlls/winebus.sys/Makefile.in > @@ -1,9 +1,10 @@ > MODULE = winebus.sys > IMPORTS = ntoskrnl setupapi > -EXTRALIBS = $(UDEV_LIBS) > +EXTRALIBS = $(IOKIT_LIBS) $(UDEV_LIBS) > EXTRAINCL = $(UDEV_CFLAGS) > EXTRADLLFLAGS = -Wb,--subsystem,native > > C_SRCS = \ > + bus_iohid.c \ > bus_udev.c \ > main.c > diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h > index 45a65e2..effbd4a 100644 > --- a/dlls/winebus.sys/bus.h > +++ b/dlls/winebus.sys/bus.h > @@ -18,6 +18,7 @@ > > /* Busses */ > NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path) DECLSPEC_HIDDEN; > +NTSTATUS WINAPI iohid_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path) DECLSPEC_HIDDEN; > > /* Native device function table */ > typedef struct > diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c > new file mode 100644 > index 0000000..e47ed13 > --- /dev/null > +++ b/dlls/winebus.sys/bus_iohid.c > @@ -0,0 +1,255 @@ > +/* Bus like function for mac HID devices > + * > + * Copyright 2016 CodeWeavers, Aric Stewart > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA > + */ > + > +#include "config.h" > +#include "wine/port.h" > + > +#include <stdarg.h> > + > +#if defined(HAVE_IOKIT_HID_IOHIDLIB_H) > +#define DWORD UInt32 > +#define LPDWORD UInt32* > +#define LONG SInt32 > +#define LPLONG SInt32* > +#define E_PENDING __carbon_E_PENDING > +#define ULONG __carbon_ULONG > +#define E_INVALIDARG __carbon_E_INVALIDARG > +#define E_OUTOFMEMORY __carbon_E_OUTOFMEMORY > +#define E_HANDLE __carbon_E_HANDLE > +#define E_ACCESSDENIED __carbon_E_ACCESSDENIED > +#define E_UNEXPECTED __carbon_E_UNEXPECTED > +#define E_FAIL __carbon_E_FAIL > +#define E_ABORT __carbon_E_ABORT > +#define E_POINTER __carbon_E_POINTER > +#define E_NOINTERFACE __carbon_E_NOINTERFACE > +#define E_NOTIMPL __carbon_E_NOTIMPL > +#define S_FALSE __carbon_S_FALSE > +#define S_OK __carbon_S_OK > +#define HRESULT_FACILITY __carbon_HRESULT_FACILITY > +#define IS_ERROR __carbon_IS_ERROR > +#define FAILED __carbon_FAILED > +#define SUCCEEDED __carbon_SUCCEEDED > +#define MAKE_HRESULT __carbon_MAKE_HRESULT > +#define HRESULT __carbon_HRESULT > +#define STDMETHODCALLTYPE __carbon_STDMETHODCALLTYPE > +#define PAGE_SHIFT __carbon_PAGE_SHIFT > +#include <IOKit/IOKitLib.h> > +#include <IOKit/hid/IOHIDLib.h> > +#undef ULONG > +#undef E_INVALIDARG > +#undef E_OUTOFMEMORY > +#undef E_HANDLE > +#undef E_ACCESSDENIED > +#undef E_UNEXPECTED > +#undef E_FAIL > +#undef E_ABORT > +#undef E_POINTER > +#undef E_NOINTERFACE > +#undef E_NOTIMPL > +#undef S_FALSE > +#undef S_OK > +#undef HRESULT_FACILITY > +#undef IS_ERROR > +#undef FAILED > +#undef SUCCEEDED > +#undef MAKE_HRESULT > +#undef HRESULT > +#undef STDMETHODCALLTYPE > +#undef DWORD > +#undef LPDWORD > +#undef LONG > +#undef LPLONG > +#undef E_PENDING > +#undef PAGE_SHIFT > +#endif /* HAVE_IOKIT_HID_IOHIDLIB_H */ > + > +#define NONAMELESSUNION > + > +#include "ntstatus.h" > +#define WIN32_NO_STATUS > +#include "windef.h" > +#include "winbase.h" > +#include "winternl.h" > +#include "winioctl.h" > +#include "ddk/wdm.h" > +#include "wine/debug.h" > + > +#include "bus.h" > + > +WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > +#ifdef HAVE_IOHIDMANAGERCREATE > + > +static DRIVER_OBJECT *iohid_driver_obj = NULL; > +static IOHIDManagerRef hid_manager; > + > +static const WCHAR busidW[] = {'I','O','H','I','D',0}; > + > +#include "initguid.h" > +DEFINE_GUID(GUID_DEVCLASS_IOHID, 0x989D309D,0x0470,0x4E1A,0x89,0x38,0x50,0x1F,0x42,0xBD,0x9A,0xCD); > + > +static void CFStringToWSTR(CFStringRef cstr, LPWSTR wstr, int length) > +{ > + int len = min(CFStringGetLength(cstr), length-1); > + CFStringGetCharacters(cstr, CFRangeMake(0, len), wstr); Are you sure it doesn't cause a compiler warning? Winemac code always uses casts from WCHAR* to UniChar*. > + wstr[len] = 0; > +} > + > +static DWORD CFNumberToDWORD(CFNumberRef num) > +{ > + int dwNum = 0; > + if (num) > + CFNumberGetValue(num, kCFNumberIntType, &dwNum); > + return dwNum; > +} > + > +static int compare_platform_device(DEVICE_OBJECT *device, void *platform_dev) > +{ > + return 0; > +} > + > +static NTSTATUS get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS begin_report_processing(DEVICE_OBJECT *device) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS set_output_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > +{ > + *written = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS get_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *read) > +{ > + *read = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS set_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > +{ > + *written = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static const platform_vtbl iohid_vtbl = { Usually "{" is on the next line. > + compare_platform_device, > + get_reportdescriptor, > + get_string, > + begin_report_processing, > + set_output_report, > + get_feature_report, > + set_feature_report, > +}; > + > +static void handle_DeviceMatchingCallback(void *inContext, IOReturn inResult, void *inSender, IOHIDDeviceRef inIOHIDDeviceRef) Those parameter names sound a bit unusual. Why is there always an "in" prefix in front? And why are there two spaces in front of "inIOHIDDeviceRef"? > +{ > + DEVICE_OBJECT *device; > + DWORD vid, pid, version; > + CFStringRef str = NULL; > + WCHAR serial_string[256]; > + BOOL is_gamepad; > + > + TRACE("OS/X IOHID Device Added %p\n", inIOHIDDeviceRef); > + > + vid = CFNumberToDWORD(IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDVendorIDKey))); > + pid = CFNumberToDWORD(IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDProductIDKey))); > + version = CFNumberToDWORD(IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDVersionNumberKey))); > + str = IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDSerialNumberKey)); > + if (str) CFStringToWSTR(str, serial_string, sizeof(serial_string) / sizeof(WCHAR)); > + > + is_gamepad = (IOHIDDeviceConformsTo(inIOHIDDeviceRef, kHIDPage_GenericDesktop, kHIDUsage_GD_GamePad) || > + IOHIDDeviceConformsTo(inIOHIDDeviceRef, kHIDPage_GenericDesktop, kHIDUsage_GD_Joystick)); > + > + device = bus_create_hid_device(iohid_driver_obj, busidW, vid, pid, version, 0, str?serial_string:NULL, is_gamepad, &GUID_DEVCLASS_IOHID, &iohid_vtbl, sizeof(inIOHIDDeviceRef)); > + if (!device) > + ERR("Failed to create device\n"); > + else > + { > + IOHIDDeviceRef* ext = get_platform_private(device); > + *ext = inIOHIDDeviceRef; I think it would be better to use a struct, even if you only want to store one value at the moment. Also, please use a helper function to avoid casts in the rest of the driver code, similar to how it is done for udev. > + IoInvalidateDeviceRelations(device, BusRelations); > + } > +} > + > +/* This puts the relevant run loop for event handling into a WINE thread */ > +static DWORD CALLBACK runloop_thread(VOID *args) Usually you should always use lowercase "void" in Wine code. > +{ > + DRIVER_OBJECT *driver = (DRIVER_OBJECT*)args; > + CFRunLoopRef run_loop = CFRunLoopGetCurrent(); > + > + IOHIDManagerSetDeviceMatching(hid_manager, NULL); > + IOHIDManagerRegisterDeviceMatchingCallback(hid_manager, handle_DeviceMatchingCallback, driver); There is no need to pass "driver" as context when you don't use it in the callback. > + IOHIDManagerScheduleWithRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode); > + if (IOHIDManagerOpen( hid_manager, 0 ) != kIOReturnSuccess) > + { > + ERR("Couldn't open IOHIDManager.\n"); > + IOHIDManagerUnscheduleFromRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode); > + CFRelease(hid_manager); > + return 0; > + } > + > + CFRunLoopRun(); > + TRACE("Run Loop exiting\n"); > + > + IOHIDManagerRegisterDeviceMatchingCallback(hid_manager, NULL, NULL); > + IOHIDManagerUnscheduleFromRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode); Can you be sure that run_loop is still valid here? > + CFRelease(hid_manager); > + return 1; > +} > + > +NTSTATUS WINAPI iohid_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path) > +{ > + HANDLE run_loop_handle; > + > + TRACE("(%p, %s)\n", driver, debugstr_w(registry_path->Buffer)); > + > + iohid_driver_obj = driver; > + driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch; > + driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch; > + hid_manager = IOHIDManagerCreate(kCFAllocatorDefault, 0L); > + if (!(run_loop_handle = CreateThread(NULL, 0, runloop_thread, driver, 0, NULL))) > + { > + ERR("Failed to initialize IOHID Manager thread\n"); > + iohid_driver_obj = NULL; > + CFRelease(hid_manager); > + return STATUS_UNSUCCESSFUL; > + } > + > + CloseHandle(run_loop_handle); > + return STATUS_SUCCESS; > +} > + > +#else > + > +NTSTATUS WINAPI iohid_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path) > +{ > + WARN("IOHID Support not compiled into Wine.\n"); > + return STATUS_NOT_IMPLEMENTED; > +} > + > +#endif /* HAVE_IOHIDMANAGERCREATE */ > diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c > index 791ccbb..ed95435 100644 > --- a/dlls/winebus.sys/main.c > +++ b/dlls/winebus.sys/main.c > @@ -628,10 +628,13 @@ NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) > { > static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0}; > static UNICODE_STRING udev = {sizeof(udevW) - sizeof(WCHAR), sizeof(udevW), (WCHAR *)udevW}; > + static const WCHAR iohidW[] = {'\\','D','r','i','v','e','r','\\','I','O','H','I','D',0}; > + static UNICODE_STRING iohid = {sizeof(iohidW) - sizeof(WCHAR), sizeof(iohidW), (WCHAR *)iohidW}; > > TRACE( "(%p, %s)\n", driver, debugstr_w(path->Buffer) ); > > IoCreateDriver(&udev, udev_driver_init); > + IoCreateDriver(&iohid, iohid_driver_init); > > return STATUS_SUCCESS; > } > > >

2 1

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

2001

wine-devel