Re: [PATCH] libudev: device - add devtype support

[Marcel Holtmann <marcel@holtmann.org>]

Hi Kay,

> >> >>> >> > I was playing a little bit with libudev and I actually need the DEVTYPE
> >> >>> >> > from uevent for various tasks. Especially with USB and Bluetooth, the
> >> >>> >> > subsystem value is too generic.
> >> >>> >> >
> >> >>> >> > Attached is a patch that implements udev_device_get_devtype() and also
> >> >>> >> > udev_device_get_parent_with_devtype(). Please double check that I did it
> >> >>> >> > the right way.
> >> >>> >>
> >> >>> >> Looks good. Applied.
> >> >>> >
> >> >>> > one minor thing that came to my mind is that DEVTYPE and subsystem are
> >> >>> > actually kinda coupled. So a DEVTYPE="host" has a different semantic for
> >> >>> > USB than for Bluetooth subsystem for example. Not sure if we actually
> >> >>> > care or just add a udev_device_get_parent_with_subsystem_devtype()
> >> >>> > function to give applications a choice if they wanna care.
> >> >>>
> >> >>> You mean replacing:
> >> >>>   udev_device_get_parent_with_devtype(..., *devtype)
> >> >>> by:
> >> >>>   udev_device_get_parent_with_subsystem_devtype(..., *subsystem,  *devtype)
> >> >>> ?
> >> >>>
> >> >>> Sounds sensible, because in most cases you don't want to check for
> >> >>> parents of a different subsystem. As you are using it, want to send a
> >> >>> patch?
> >> >>
> >> >> I am thinking of keeping both. So just adding ...subsystem_devtype() and
> >> >> keeping also the original one. My reason for it is that in some case you
> >> >> already know the subsystem you are looking at (or don't care in). So no
> >> >> point in doing a lookup with two checks.
> >> >
> >> > We could make it accept NULL for the subsystem in that case?
> >>
> >> I guess, we should just have
> >> udev_device_get_parent_with_subsystem_devtype(), and drop the both
> >> other ones, and accept NULL as parameters, if you want to match only
> >> one. You are right that subsystem and devtype belong together, so we
> >> should probably reflect that in the API. Any problems with such
> >> change?
> >
> > I think that make sense. So I opted for allowing devtype being NULL
> > while subsystem is required. I tried to come up with a good reason why
> > you would look for devtype and not subsystem and couldn't find it right
> > now. The USB case is double information since they prefixed their
> > devtypes with "usb_", but that is really an USB subsystem problem. For
> > Bluetooth I am not doing this at all and neither does SCSI. In future no
> > subsystem should.
> 
> Yeah, sounds all fine. Patches are applied.

you even fixed my typo. Just found that one a few minutes ago. Thanks :)

Regards

Marcel