February 2017 - Wine-Devel - list.winehq.org

Re: [PATCH 2/2] wineinstall - Add support for parallel builds using 'nproc' to detect cpu count

by Austin English

On Mon, Feb 27, 2017 at 8:20 PM, Rosanne DiMesio <dimesio(a)earthlink.net> wrote: > On Mon, 27 Feb 2017 18:53:21 -0500 > Chris Morgan <chmorgan(a)gmail.com> wrote: > >> >> wineinstall still serves a pretty useful purpose of making it very easy for >> people to get started building wine. > > Not in my experience. It does not warn users about missing dependencies, and they show up on the forum wondering why the Wine they built doesn't work. I advise users against using it, and would like nothing better than for it to go away. > > > -- > Rosanne DiMesio > dimesio(a)earthlink.net https://source.winehq.org/patches/data/130850 -- -Austin GPG: 14FB D7EA A041 937B

Feb. 28, 2017

Re: [PATCH 2/2] wineinstall - Add support for parallel builds using 'nproc' to detect cpu count

by Rosanne DiMesio

On Mon, 27 Feb 2017 18:53:21 -0500 Chris Morgan <chmorgan(a)gmail.com> wrote: > > wineinstall still serves a pretty useful purpose of making it very easy for > people to get started building wine. Not in my experience. It does not warn users about missing dependencies, and they show up on the forum wondering why the Wine they built doesn't work. I advise users against using it, and would like nothing better than for it to go away. -- Rosanne DiMesio dimesio(a)earthlink.net

Feb. 28, 2017

Re: [PATCH 2/2] wineinstall - Add support for parallel builds using 'nproc' to detect cpu count

by Chris Morgan

On Sun, Feb 26, 2017 at 4:01 PM, Alexandre Julliard <julliard(a)winehq.org> wrote: > Sebastian Lackner <sebastian(a)fds-team.de> writes: > > > On 25.02.2017 01:40, Austin English wrote: > >> On Fri, Feb 24, 2017 at 6:30 PM, Chris Morgan <chmorgan(a)gmail.com> > wrote: > >>> Parallel builds can greatly reduce the overall build time on modern > multi-core processors. > >>> > >>> Fall back to two parallel builds in the case where nproc is > unavailable, most modern > >>> processors have at least two cores. > >>> > >>> Use the 'NPROC' value defined by the environment if it is not null. > >>> > >>> Signed-off-by: Chris Morgan <chmorgan(a)gmail.com> > >>> --- > >>> tools/wineinstall | 13 +++++++++++-- > >>> 1 file changed, 11 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/tools/wineinstall b/tools/wineinstall > >>> index bfd62e3..080d590 100755 > >>> --- a/tools/wineinstall > >>> +++ b/tools/wineinstall > >>> @@ -143,10 +143,19 @@ echo "in the meantime..." > >>> echo > >>> std_sleep > >>> > >>> +# If the NPROC environment variable isn't set use nproc, if > available, to determine > >>> +# the number of processors in the system, if not, fall back to a > default NPROC value > >>> +if type nproc2 &>/dev/null > >> > >> I don't think this will do what you want ;) > >> > >> Also, I'm pretty sure &> isn't Posix. You'd want something like: > >> if type nproc > /dev/null 2>&1 > > > > Actually, shouldn't we first discuss if we really want to keep > wineinstall as part of the > > repo before we merge any improvements? I don't know how many people are > using it, but it > > lacks many important features. It does not install any dependencies, > does not warn about > > problems during configure, and also does not support a proper wow64 > build, ... Basically, > > it could be replaced by a one-liner "./configure && make && su root -c > 'make install'"... > > Do we really want to keep it? > > I didn't mind keeping it as long as it was just sitting there, but if we > are going to start adding complexity or bikeshedding it, then it's > probably better to remove it. > > -- > Alexandre Julliard > julliard(a)winehq.org > wineinstall still serves a pretty useful purpose of making it very easy for people to get started building wine. We can update documentations or let users hunt around to figure out their distro specific ways of running make install or providing commands to check for potentially conflicting existing binaries of wine. Those are useful things even for people that are very familiar with building from source code. It can't be directly replaced by a one liner in most cases. I'd recommend merging in my fixes and letting people use wineinstall to get started in building wine. I'm all for adding distro specific dependencies. For fedora it was a lot of trouble finding dependencies. I don't see a good way to do the equivalent of 'apt-get build-dep wine' on fedora, so in that case it would be quite useful to help the user out with a list of dependencies, for debian maybe prompt or run 'apt-get build-dep wine' etc. Chris

Feb. 27, 2017

Re: Proposal: winetest 'context'

by Sebastian Lackner

On 27.02.2017 20:19, Mark Jansen wrote: > This syntax is a lot better, > however it will break when ':' is used in the format string. > It might be a better idea to keep a list of indexes alongside the > context buffer? Not sure if its required. I think it would be a bit confusing when ":" is used inside of a context string. If devs want to use ":" as separator for variables they could also do something like: with_context("i=%d", i) with_context("j=%d", j) { // here some tests } > > As for the name, use_context or set_context ? That would also be fine, but I don't think its much better than with_context(). > > regards, > > Mark Jansen > > On Mon, Feb 27, 2017 at 12:06 AM, Sebastian Lackner > <sebastian(a)fds-team.de> wrote: >> On 25.02.2017 13:34, Józef Kucia wrote: >>> On Tue, Feb 14, 2017 at 5:04 PM, Mark Jansen <learn0more+wine(a)gmail.com> wrote: >>>> Hello, >>>> >>>> Often when testing something, helper functions are introduced that are >>>> called with different data (for example when iterating over tables). >>>> >>>> Attached patch (winetest_set_context.diff) is a proposal that allows >>>> tests to specify a 'context', that will be printed in the failure >>>> message. >>>> This way, the actual test code does not have to prefix every case with >>>> a trace, or include the data in every test macro. >>>> The patch is written so that tests do not have to use this mechanism, >>>> so there is no need to convert all tests at once. >>>> Second attachment (winetest_ctx_example.diff) shows how an existing >>>> test could be converted to use this new technique. >>> >>> For what it's worth, if we had a mechanism to set a context for test >>> messages, I would gladly use it in d3d11 tests. >>> >>> Cheers, >>> Józef >> >> I would also like to see such a feature, however I'm not really happy with >> a single context string which has to be set and reset manually. So, here is >> an alternative proposal which allows to use a block-syntax like: >> >> --- snip --- >> with_context("i = %d, j = %d", i, j) >> { >> // here some tests >> } >> --- snip --- >> >> Blocks can also be nested, and its easy to combine them with a loop: >> >> --- snip --- >> for (i = 0; i < 100; i++) with_context("test %d", i) >> { >> // here some tests (don't use break!) >> } >> --- snip --- >> >> The only disadvantage is that we need variadic macros (__VA_ARGS__), but it >> looks like they are already used in a bunch of other tests, so thats probably >> not a real problem. Please let me know what you think about this method, and >> also if you have suggestions for a better name. >> >> Best regards, >> Sebastian >>

Feb. 27, 2017

Re: [PATCH 4/5] server: Added server side mamed pipe read and write implementation and use it for message mode pipes.

by Sebastian Lackner

On 22.02.2017 14:51, Jacek Caban wrote: > Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> > --- > dlls/kernel32/tests/pipe.c | 28 ++- > dlls/ntdll/file.c | 7 +- > dlls/ntdll/tests/file.c | 2 +- > server/async.c | 9 + > server/file.h | 1 + > server/named_pipe.c | 446 > ++++++++++++++++++++++++++++++++++++++++++--- > 6 files changed, 447 insertions(+), 46 deletions(-) As I see this updated patchset allows to return the result immediately, but still uses a USR1 signal to transfer back the IO completion info. Wouldn't it be useful to get rid of that aswell? Also please note that we need to distinguish "immediate returns" and "async returns" to properly implement FileIoCompletionNotificationInformation. This especially means that waiting internally is not an option. > + iosb = async_get_iosb( async ); > + if ((blocking || iosb->status != STATUS_PENDING) > + && !(handle = alloc_handle( current->process, async, SYNCHRONIZE, 0 ))) > + async_terminate( async, get_error() ); I don't think this will work as expected. async_terminate might queue an APC, but the caller will immediately deallocate the async struct in server_write_file because status != STATUS_PENDING.

Feb. 27, 2017

The Wine Project has been accepted as a GSoC 2017 mentor organization!

by André Hentschel

I just invited a lot of possible Mentors, if I missed someone, please let me know

Feb. 27, 2017

Re: [1/4] dinput: Keep username same between device objects.

by Jetro Jormalainen

Hi These patches are step towards fixing this bug: https://bugs.winehq.org/show_bug.cgi?id=34108 You can use Rally Trophy Demo to test. Currently keyboard/joystick keys dont respond at all while playing without this patch. Sun, 26 Feb 2017 23:41:26 -0300 Bruno Jesus <00cpxxx(a)gmail.com> kirjoitti: > On Wed, Feb 22, 2017 at 8:22 PM, Jetro Jormalainen > <jje-wine(a)jv.jetro.fi> wrote: > > Tested on Windows 10, Windows XP and Arch Linux. > > Hi, thanks for the patches. In order to help testing do you mind > explaining how to test them? Like is there any game demo/freeware that > is able to benefit from the patches?

Feb. 27, 2017

Re: [PATCH 1/5] winebus.sys: Handle linux input event device via udev

by Aric Stewart

On 2/27/17 10:52 AM, Sebastian Lackner wrote: > On 27.02.2017 17:22, Aric Stewart wrote: >> On 2/27/17 10:00 AM, Sebastian Lackner wrote: >>> I assume this is meant as a fallback, if we don't have access to >>> hidraw, right? If you plan to use this as a replacement for dinput, >>> it would be better to implement it without fixed dependency on >>> udev. >>> >> Likely users using HID for gamepads and joysticks will end up this >> route by default because all the permissions are automatically setup >> for us. Also then user will get to leverage all the work done in the >> kernel to handle devices that do not behave well on the HID side or >> which do not show up at all using hidraw (Xbox Controllers for >> example) >> >> The dependency for udev is for common device discovery. I feel like I >> still want that dependency unless we write our own device plug and >> unplug discovery system. > > Are you sure that there are no systems out there where we have linux > events, but no udev? If yes the current solution is fine, but otherwise > we should at least implement a fallback method similar to how its done > in dinput (basically, just trying to open all devices in the range 0 .. > MAX_JOYDEV). > I am not a linux expert but according to quick web research udev has been default since about Linux kernel version 2.6.13. That is sometime around 2006. It became part of systemd in 2012. So I am reasonably sure that any modern linux system will have it. I am not sure how much we worry about pre 2006 systems. -aric >>> Does it hurt when both hidraw and input event devices are found? If >>> yes, it would be better to filter duplicates somewhere else, >>> instead of using registry keys. >>> >> You can get duplicate devices, on being talking to the hidraw device >> and one to the linux event device, but both are the same physical >> device. I have explored a little bit to try to find if there is a >> good way to detect if a device is duplicated but it has not proven to >> be immediately easy. > > If it can cause problems it is probably still worth the effort. There > might also be systems where hidraw devices have access permissions by > default, and we don't want to get duplicated input events then. > >> Also then the question because which device is >> preferred if we find a duplicate. >> > > I think preferring hidraw might be slightly better. Some devices also > offer their own control utilities, and those would probably not work when > using linux input events. Nevertheless, its a matter of taste and we can > certainly discuss about that. > > Best regards, > Sebastian >

Feb. 27, 2017

Re: [PATCH 1/5] winebus.sys: Handle linux input event device via udev

by Sebastian Lackner

On 27.02.2017 17:22, Aric Stewart wrote: > On 2/27/17 10:00 AM, Sebastian Lackner wrote: >> I assume this is meant as a fallback, if we don't have access to >> hidraw, right? If you plan to use this as a replacement for dinput, >> it would be better to implement it without fixed dependency on >> udev. >> > Likely users using HID for gamepads and joysticks will end up this > route by default because all the permissions are automatically setup > for us. Also then user will get to leverage all the work done in the > kernel to handle devices that do not behave well on the HID side or > which do not show up at all using hidraw (Xbox Controllers for > example) > > The dependency for udev is for common device discovery. I feel like I > still want that dependency unless we write our own device plug and > unplug discovery system. Are you sure that there are no systems out there where we have linux events, but no udev? If yes the current solution is fine, but otherwise we should at least implement a fallback method similar to how its done in dinput (basically, just trying to open all devices in the range 0 .. MAX_JOYDEV). >> Does it hurt when both hidraw and input event devices are found? If >> yes, it would be better to filter duplicates somewhere else, >> instead of using registry keys. >> > You can get duplicate devices, on being talking to the hidraw device > and one to the linux event device, but both are the same physical > device. I have explored a little bit to try to find if there is a > good way to detect if a device is duplicated but it has not proven to > be immediately easy. If it can cause problems it is probably still worth the effort. There might also be systems where hidraw devices have access permissions by default, and we don't want to get duplicated input events then. > Also then the question because which device is > preferred if we find a duplicate. > I think preferring hidraw might be slightly better. Some devices also offer their own control utilities, and those would probably not work when using linux input events. Nevertheless, its a matter of taste and we can certainly discuss about that. Best regards, Sebastian

Feb. 27, 2017

Re: [PATCH 1/5] winebus.sys: Handle linux input event device via udev

by Aric Stewart

Thanks for the review! On 2/27/17 10:00 AM, Sebastian Lackner wrote: > On 27.02.2017 16:09, Aric Stewart wrote: >> Signed-off-by: Aric Stewart <aric(a)codeweavers.com> >> --- >> dlls/winebus.sys/bus.h | 3 + >> dlls/winebus.sys/bus_udev.c | 150 ++++++++++++++++++++++++++++++++++++++++++-- >> dlls/winebus.sys/main.c | 26 ++++++++ >> 3 files changed, 173 insertions(+), 6 deletions(-) >> > > I assume this is meant as a fallback, if we don't have access to hidraw, right? > If you plan to use this as a replacement for dinput, it would be better to > implement it without fixed dependency on udev. > Likely users using HID for gamepads and joysticks will end up this route by default because all the permissions are automatically setup for us. Also then user will get to leverage all the work done in the kernel to handle devices that do not behave well on the HID side or which do not show up at all using hidraw (Xbox Controllers for example) The dependency for udev is for common device discovery. I feel like I still want that dependency unless we write our own device plug and unplug discovery system. >> >> >> 0001-winebus.sys-Handle-linux-input-event-device-via-udev.txt >> >> >> diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h >> index effbd4afd0..fed35e8c1e 100644 >> --- a/dlls/winebus.sys/bus.h >> +++ b/dlls/winebus.sys/bus.h >> @@ -43,3 +43,6 @@ DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev >> void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; >> NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; >> void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD length) DECLSPEC_HIDDEN; >> + >> +/* General Bus Functions */ >> +DWORD check_bus_option(UNICODE_STRING *registry_path, const UNICODE_STRING *option) DECLSPEC_HIDDEN; >> diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c >> index 415bd3db08..2dcc7fb442 100644 >> --- a/dlls/winebus.sys/bus_udev.c >> +++ b/dlls/winebus.sys/bus_udev.c >> @@ -42,6 +42,14 @@ >> # include <sys/ioctl.h> >> #endif >> >> +#ifdef HAVE_LINUX_INPUT_H >> +# include <linux/input.h> >> +# undef SW_MAX >> +# if defined(EVIOCGBIT) && defined(EV_ABS) && defined(BTN_PINKIE) >> +# define HAS_PROPER_INPUT_HEADER >> +# endif >> +#endif >> + >> #define NONAMELESSUNION >> >> #include "ntstatus.h" >> @@ -65,11 +73,15 @@ WINE_DECLARE_DEBUG_CHANNEL(hid_report); >> >> static struct udev *udev_context = NULL; >> static DRIVER_OBJECT *udev_driver_obj = NULL; >> +static DWORD disable_hidraw = 0; >> +static DWORD disable_input = 0; >> >> static const WCHAR hidraw_busidW[] = {'H','I','D','R','A','W',0}; >> +static const WCHAR lnxev_busidW[] = {'L','N','X','E','V',0}; >> >> #include "initguid.h" >> DEFINE_GUID(GUID_DEVCLASS_HIDRAW, 0x3def44ad,0x242e,0x46e5,0x82,0x6d,0x70,0x72,0x13,0xf3,0xaa,0x81); >> +DEFINE_GUID(GUID_DEVCLASS_LINUXEVENT, 0x1b932c0d,0xfea7,0x42cd,0x8e,0xaa,0x0e,0x48,0x79,0xb6,0x9e,0xaa); >> >> struct platform_private >> { >> @@ -392,6 +404,52 @@ static const platform_vtbl hidraw_vtbl = >> hidraw_set_feature_report, >> }; >> >> +#ifdef HAS_PROPER_INPUT_HEADER >> + >> +static NTSTATUS lnxev_get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length) >> +{ >> + return STATUS_NOT_IMPLEMENTED; >> +} >> + >> +static NTSTATUS lnxev_get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length) >> +{ >> + return STATUS_NOT_IMPLEMENTED; >> +} >> + >> +static NTSTATUS lnxev_begin_report_processing(DEVICE_OBJECT *device) >> +{ >> + return STATUS_NOT_IMPLEMENTED; >> +} >> + >> +static NTSTATUS lnxev_set_output_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) >> +{ >> + *written = 0; >> + return STATUS_NOT_IMPLEMENTED; >> +} >> + >> +static NTSTATUS lnxev_get_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *read) >> +{ >> + *read = 0; >> + return STATUS_NOT_IMPLEMENTED; >> +} >> + >> +static NTSTATUS lnxev_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 lnxev_vtbl = { >> + compare_platform_device, >> + lnxev_get_reportdescriptor, >> + lnxev_get_string, >> + lnxev_begin_report_processing, >> + lnxev_set_output_report, >> + lnxev_get_feature_report, >> + lnxev_set_feature_report, >> +}; >> +#endif >> + >> static void try_add_device(struct udev_device *dev) >> { >> DWORD vid = 0, pid = 0, version = 0; >> @@ -400,6 +458,7 @@ static void try_add_device(struct udev_device *dev) >> const char *subsystem; >> const char *devnode; >> WCHAR *serial = NULL; >> + const char* gamepad = NULL; >> int fd; >> >> if (!(devnode = udev_device_get_devnode(dev))) >> @@ -419,6 +478,26 @@ static void try_add_device(struct udev_device *dev) >> version = get_sysattr_dword(usbdev, "version", 10); >> serial = get_sysattr_string(usbdev, "serial"); >> } >> +#ifdef HAS_PROPER_INPUT_HEADER >> + else >> + { >> + struct input_id device_id = {0}; >> + char device_uid[255] = {0}; >> + >> + if (ioctl(fd, EVIOCGID, &device_id) == -1) >> + WARN("ioctl(EVIOCGID) failed: %d %s\n", errno, strerror(errno)); >> + if (ioctl(fd, EVIOCGUNIQ(254), device_uid) && device_uid[0]) >> + MultiByteToWideChar(CP_UNIXCP, 0, device_uid, -1, serial, 255); > > I don't think this will work as expected. serial is a NULL pointer, and the check > if ioctl() was successful is also done wrong. On error a value < 0 is returned. Thanks I will look at that. > >> + >> + gamepad = udev_device_get_property_value(dev, "ID_INPUT_JOYSTICK"); >> + vid = device_id.vendor; >> + pid = device_id.product; >> + version = device_id.version; >> + } >> +#else >> + else >> + WARN("Could not get device to query VID, PID, Version and Serial\n"); >> +#endif >> >> TRACE("Found udev device %s (vid %04x, pid %04x, version %u, serial %s)\n", >> debugstr_a(devnode), vid, pid, version, debugstr_w(serial)); >> @@ -429,6 +508,12 @@ static void try_add_device(struct udev_device *dev) >> device = bus_create_hid_device(udev_driver_obj, hidraw_busidW, vid, pid, version, 0, serial, FALSE, >> &GUID_DEVCLASS_HIDRAW, &hidraw_vtbl, sizeof(struct platform_private)); >> } >> +#ifdef HAS_PROPER_INPUT_HEADER >> + else if (strcmp(subsystem, "input") == 0) >> + { >> + device = bus_create_hid_device(udev_driver_obj, lnxev_busidW, vid, pid, version, 0, serial, (gamepad != NULL), &GUID_DEVCLASS_LINUXEVENT, &lnxev_vtbl, sizeof(struct platform_private)); > > Some linebreaks wouldn't hurt. :) > >> + } >> +#endif >> >> if (device) >> { >> @@ -448,8 +533,17 @@ static void try_add_device(struct udev_device *dev) >> >> static void try_remove_device(struct udev_device *dev) >> { >> - DEVICE_OBJECT *device = bus_find_hid_device(&hidraw_vtbl, dev); >> - struct platform_private *private; >> + DEVICE_OBJECT *device = NULL; >> + const char* subsystem; >> + struct platform_private* private; >> + >> + subsystem = udev_device_get_subsystem(dev); >> + if (strcmp(subsystem, "hidraw") == 0) >> + device = bus_find_hid_device(&hidraw_vtbl, dev); >> +#ifdef HAS_PROPER_INPUT_HEADER >> + else if (strcmp(subsystem, "input") == 0) >> + device = bus_find_hid_device(&lnxev_vtbl, dev); >> +#endif > > It might be easier to just check both after each other, like: > > device = bus_find_hid_device(...); > if (!device) device = bus_find_hid_device(...); > either way, seems mostly stylistic >> if (!device) return; >> >> IoInvalidateDeviceRelations(device, RemovalRelations); >> @@ -483,8 +577,18 @@ static void build_initial_deviceset(void) >> return; >> } >> >> - if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0) >> - WARN("Failed to add subsystem 'hidraw' to enumeration\n"); >> + if (!disable_hidraw) >> + if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0) >> + WARN("Failed to add subsystem 'hidraw' to enumeration\n"); >> +#ifdef HAS_PROPER_INPUT_HEADER >> + if (!disable_input) >> + { >> + if (udev_enumerate_add_match_subsystem(enumerate, "event") < 0) >> + WARN("Failed to add subsystem 'event' to enumeration\n"); > > Why do you add this match when results are not processed anywhere? legacy code. I will remove it. > >> + if (udev_enumerate_add_match_subsystem(enumerate, "input") < 0) >> + WARN("Failed to add subsystem 'input' to enumeration\n"); >> + } >> +#endif >> >> if (udev_enumerate_scan_devices(enumerate) < 0) >> WARN("Enumeration scan failed\n"); >> @@ -509,6 +613,7 @@ static void build_initial_deviceset(void) >> static struct udev_monitor *create_monitor(struct pollfd *pfd) >> { >> struct udev_monitor *monitor; >> + int systems = 0; >> >> monitor = udev_monitor_new_from_netlink(udev_context, "udev"); >> if (!monitor) >> @@ -517,8 +622,27 @@ static struct udev_monitor *create_monitor(struct pollfd *pfd) >> return NULL; >> } >> >> - if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "hidraw", NULL) < 0) >> - WARN("Failed to add subsystem 'hidraw' to monitor\n"); >> + if (!disable_hidraw) >> + { >> + if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "hidraw", NULL) < 0) >> + WARN("Failed to add 'hidraw' subsystem to monitor\n"); >> + else >> + systems++; >> + } >> +#ifdef HAS_PROPER_INPUT_HEADER >> + if (!disable_input) >> + { >> + if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "input", NULL) < 0) >> + WARN("Failed to add 'input' subsystem to monitor\n"); >> + else >> + systems++; >> + } >> +#endif >> + if (systems == 0) >> + { >> + WARN("No subsystems added to monitor\n"); >> + goto error; >> + } >> >> if (udev_monitor_enable_receiving(monitor) < 0) >> goto error; >> @@ -589,6 +713,10 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry >> { >> HANDLE events[2]; >> DWORD result; >> + static const WCHAR hidraw_disabledW[] = {'D','i','s','a','b','l','e',' ','h','i','d','r','a','w',0}; >> + static const UNICODE_STRING hidraw_disabled = {sizeof(hidraw_disabledW) - sizeof(WCHAR), sizeof(hidraw_disabledW), (WCHAR*)hidraw_disabledW}; >> + static const WCHAR input_disabledW[] = {'D','i','s','a','b','l','e',' ','i','n','p','u','t',0}; >> + static const UNICODE_STRING input_disabled = {sizeof(input_disabledW) - sizeof(WCHAR), sizeof(input_disabledW), (WCHAR*)input_disabledW}; >> >> TRACE("(%p, %s)\n", driver, debugstr_w(registry_path->Buffer)); >> >> @@ -602,6 +730,16 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry >> driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch; >> driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch; >> >> + disable_hidraw = check_bus_option(registry_path, &hidraw_disabled); >> + if (disable_hidraw) >> + TRACE("UDEV hidraw devices disabled in registry\n"); >> + >> +#ifdef HAS_PROPER_INPUT_HEADER >> + disable_input = check_bus_option(registry_path, &input_disabled); >> + if (disable_input) >> + TRACE("UDEV input devices disabled in registry\n"); >> +#endif > > Does it hurt when both hidraw and input event devices are found? If yes, it > would be better to filter duplicates somewhere else, instead of using registry > keys. > You can get duplicate devices, on being talking to the hidraw device and one to the linux event device, but both are the same physical device. I have explored a little bit to try to find if there is a good way to detect if a device is duplicated but it has not proven to be immediately easy. Also then the question because which device is preferred if we find a duplicate. thanks again for the review, I am seeing that I have been testing with either or and not both (hidraw/event) so you are finding very valid issues in the code I need to resolve. I was starting to go cross-eyed looking at it all. -aric >> + >> if (!(events[0] = CreateEventW(NULL, TRUE, FALSE, NULL))) >> goto error; >> if (!(events[1] = CreateThread(NULL, 0, deviceloop_thread, events[0], 0, NULL))) >> diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c >> index 978540bb17..0388f0263c 100644 >> --- a/dlls/winebus.sys/main.c >> +++ b/dlls/winebus.sys/main.c >> @@ -625,6 +625,32 @@ void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD length) >> LeaveCriticalSection(&ext->report_cs); >> } >> >> +DWORD check_bus_option(UNICODE_STRING *registry_path, const UNICODE_STRING *option) >> +{ >> + OBJECT_ATTRIBUTES attr; >> + HANDLE key; >> + DWORD output = 0; >> + >> + InitializeObjectAttributes(&attr, registry_path, OBJ_CASE_INSENSITIVE | OBJ_KERNEL_HANDLE, NULL, NULL); >> + if (NtOpenKey(&key, KEY_ALL_ACCESS, &attr) == STATUS_SUCCESS) >> + { >> + DWORD size; >> + char buffer[FIELD_OFFSET(KEY_VALUE_PARTIAL_INFORMATION, Data[sizeof(DWORD)])]; >> + >> + KEY_VALUE_PARTIAL_INFORMATION *info = (KEY_VALUE_PARTIAL_INFORMATION*)buffer; >> + >> + if (NtQueryValueKey(key, option, KeyValuePartialInformation, info, sizeof(buffer), &size) == STATUS_SUCCESS) >> + { >> + if (info->Type == REG_DWORD) >> + output = *(DWORD*)info->Data; >> + } >> + >> + NtClose(key); >> + } >> + >> + return output; >> +} >> + >> NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) >> { >> static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0}; >> >> >> >

Feb. 27, 2017