[PATCH v7] winebus.sys: Build initial hidraw device set

[Sebastian Lackner]

On 12.09.2016 14:41, Aric Stewart wrote:
>> > 
>> > Does this work properly when elements are deleted? You might return the same index
>> > for different devices.
> As far as I understand, this index does not need to be unique to a device, just unique for in given device set of active devices, so this should be ok.

I don't think its guaranteed to be unique at all. Lets assume we have three
devices with the same vidpid (index 1, 2, 3). The second one gets removed
(index 1, 3) and a fourth one gets added. It will then attempt to use index
3 which is still in use. Is that not a problem?

>> > At least for udev the device object has to be released. It looks like the logic
>> > is currently missing (also in your draft patches). In order to fix that it will
>> > probably be necessary to store additional information in each device object.
>> > 
> That is coming in a later patch where I handle device removals. You are right that part of that will need to be moved here for the error case.

If it was part of your draft series I must have missed it. How exactly are you
planning to implement it? Does it make sense to store a release function as part
of each device, or maybe even a Vtable?

>> > How does this correspond to similar code in hidclass.sys/device.c. Do we really need both? Why does
>> > hidclass contain a call to SetupDiCreateDeviceInterfaceW but not this version?
>> > 
> These part of registry manipulations are done on windows via device installation. So setupapi is used to make these keys and then they are just expected to be there for installed devices. Since we are trying to handle native devices that have not been installed in this method we have to add the proper registry information more dynamically. That both areas have to do similar things that is why the code is similar.
> 
> These bus devices do not have any Interfaces, that is why we have no call to SetupDiCreateDeviceInterfaceW.

Okay, thanks for the explanation. I still find it a bit odd when we have all
this code with a similar purpose at different places though, especially when
it looks very different. The hidclass version for example will leak the devinfo
handle on various error paths and returns ERROR_* codes instead of NTSTATUS
values. This version here might have other issues (at least missing error
handling for some of the functions). If the code duplication cannot be avoided,
it might at least be useful to keep the code a bit more synchronized to these
other versions (after fixing them, of course).

> 
> 
> I will work on integrating all the other changes. thanks!
> 
> -aric
> 

Regards,
Sebastian