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.
So patch 0001 renames ..with_devtype() into ..with_subsystem_devtype()
and then patch 0002 removes ..with_subsystem() and replaces its usage.
Regards
Marcel
>From 53eb85ecb5bff49449b79174734f7d32d85c5af6 Mon Sep 17 00:00:00 2001
From: Marcel Holtmann <marcel@xxxxxxxxxxxx>
Date: Sat, 3 Jan 2009 11:10:10 +0100
Subject: [PATCH] libudev: device - lookup subsystem and devtype together
---
udev/lib/exported_symbols | 2 +-
udev/lib/libudev-device.c | 17 +++++++++++++----
udev/lib/libudev.h | 2 +-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/udev/lib/exported_symbols b/udev/lib/exported_symbols
index 6b07842..88d5c48 100644
--- a/udev/lib/exported_symbols
+++ b/udev/lib/exported_symbols
@@ -17,7 +17,7 @@ udev_device_new_from_devnum
udev_device_new_from_subsystem_sysname
udev_device_get_parent
udev_device_get_parent_with_subsystem
-udev_device_get_parent_with_devtype
+udev_device_get_parent_with_subsystem_devtype
udev_device_ref
udev_device_unref
udev_device_get_udev
diff --git a/udev/lib/libudev-device.c b/udev/lib/libudev-device.c
index a25716d..7c80359 100644
--- a/udev/lib/libudev-device.c
+++ b/udev/lib/libudev-device.c
@@ -557,17 +557,26 @@ struct udev_device *udev_device_get_parent_with_subsystem(struct udev_device *ud
return parent;
}
-struct udev_device *udev_device_get_parent_with_devtype(struct udev_device *udev_device, const char *devtype)
+struct udev_device *udev_device_get_parent_with_subsystem_devtype(struct udev_device *udev_device, const char *subsystem, const char *devtype)
{
struct udev_device *parent;
+ if (subsystem == NULL)
+ return NULL;
+
parent = udev_device_get_parent(udev_device);
while (parent != NULL) {
+ const char *parent_subsystem;
const char *parent_devtype;
- parent_devtype = udev_device_get_devtype(parent);
- if (parent_devtype != NULL && strcmp(parent_devtype, devtype) == 0)
- break;
+ parent_subsystem = udev_device_get_subsystem(parent);
+ if (parent_subsystem != NULL && strcmp(parent_subsystem, subsystem) == 0) {
+ if (devtype == NULL)
+ break;
+ parent_devtype = udev_device_get_devtype(parent);
+ if (parent_devtype != NULL && strcmp(parent_devtype, devtype) == 0)
+ break;
+ }
parent = udev_device_get_parent(parent);
}
return parent;
diff --git a/udev/lib/libudev.h b/udev/lib/libudev.h
index b96e494..bac362a 100644
--- a/udev/lib/libudev.h
+++ b/udev/lib/libudev.h
@@ -63,7 +63,7 @@ extern struct udev_device *udev_device_new_from_devnum(struct udev *udev, char t
extern struct udev_device *udev_device_new_from_subsystem_sysname(struct udev *udev, const char *subsystem, const char *sysname);
extern struct udev_device *udev_device_get_parent(struct udev_device *udev_device);
extern struct udev_device *udev_device_get_parent_with_subsystem(struct udev_device *udev_device, const char *subsystem);
-extern struct udev_device *udev_device_get_parent_with_devtype(struct udev_device *udev_device, const char *devtype);
+extern struct udev_device *udev_device_get_parent_with_subsytem_devtype(struct udev_device *udev_device, const char *subsystem, const char *devtype);
extern struct udev_device *udev_device_ref(struct udev_device *udev_device);
extern void udev_device_unref(struct udev_device *udev_device);
extern struct udev *udev_device_get_udev(struct udev_device *udev_device);
--
1.5.6.3
>From 9ad3a8779559099a0b55c361b46a861a407786db Mon Sep 17 00:00:00 2001
From: Marcel Holtmann <marcel@xxxxxxxxxxxx>
Date: Sat, 3 Jan 2009 11:13:51 +0100
Subject: [PATCH] libudev: device - remove udev_device_get_parent_with_subsystem
---
extras/usb_id/usb_id.c | 6 +++---
udev/lib/exported_symbols | 1 -
udev/lib/libudev-device.c | 16 ----------------
udev/lib/libudev.h | 1 -
4 files changed, 3 insertions(+), 21 deletions(-)
diff --git a/extras/usb_id/usb_id.c b/extras/usb_id/usb_id.c
index 28352eb..9ee4e6e 100644
--- a/extras/usb_id/usb_id.c
+++ b/extras/usb_id/usb_id.c
@@ -192,7 +192,7 @@ static int usb_id(struct udev_device *dev)
dbg(udev, "syspath %s\n", udev_device_get_syspath(dev));
/* usb interface directory */
- dev_interface = udev_device_get_parent_with_subsystem(dev, "usb");
+ dev_interface = udev_device_get_parent_with_subsystem_devtype(dev, "usb", NULL);
if (dev_interface == NULL) {
info(udev, "unable to access usb_interface device of '%s'\n",
udev_device_get_syspath(dev));
@@ -218,7 +218,7 @@ static int usb_id(struct udev_device *dev)
udev_device_get_syspath(dev_interface), if_class_num, protocol);
/* usb device directory */
- dev_usb = udev_device_get_parent_with_subsystem(dev_interface, "usb");
+ dev_usb = udev_device_get_parent_with_subsystem_devtype(dev_interface, "usb", NULL);
if (!dev_usb) {
info(udev, "unable to find parent 'usb' device of '%s'\n",
udev_device_get_syspath(dev));
@@ -232,7 +232,7 @@ static int usb_id(struct udev_device *dev)
int host, bus, target, lun;
/* get scsi device */
- dev_scsi = udev_device_get_parent_with_subsystem(dev, "scsi");
+ dev_scsi = udev_device_get_parent_with_subsystem_devtype(dev, "scsi", NULL);
if (dev_scsi == NULL) {
info(udev, "unable to find parent 'scsi' device of '%s'\n",
udev_device_get_syspath(dev));
diff --git a/udev/lib/exported_symbols b/udev/lib/exported_symbols
index 88d5c48..abf5051 100644
--- a/udev/lib/exported_symbols
+++ b/udev/lib/exported_symbols
@@ -16,7 +16,6 @@ udev_device_new_from_syspath
udev_device_new_from_devnum
udev_device_new_from_subsystem_sysname
udev_device_get_parent
-udev_device_get_parent_with_subsystem
udev_device_get_parent_with_subsystem_devtype
udev_device_ref
udev_device_unref
diff --git a/udev/lib/libudev-device.c b/udev/lib/libudev-device.c
index 7c80359..0cb5921 100644
--- a/udev/lib/libudev-device.c
+++ b/udev/lib/libudev-device.c
@@ -541,22 +541,6 @@ struct udev_device *udev_device_get_parent(struct udev_device *udev_device)
return udev_device->parent_device;
}
-struct udev_device *udev_device_get_parent_with_subsystem(struct udev_device *udev_device, const char *subsystem)
-{
- struct udev_device *parent;
-
- parent = udev_device_get_parent(udev_device);
- while (parent != NULL) {
- const char *parent_subsystem;
-
- parent_subsystem = udev_device_get_subsystem(parent);
- if (parent_subsystem != NULL && strcmp(parent_subsystem, subsystem) == 0)
- break;
- parent = udev_device_get_parent(parent);
- }
- return parent;
-}
-
struct udev_device *udev_device_get_parent_with_subsystem_devtype(struct udev_device *udev_device, const char *subsystem, const char *devtype)
{
struct udev_device *parent;
diff --git a/udev/lib/libudev.h b/udev/lib/libudev.h
index bac362a..63b9b79 100644
--- a/udev/lib/libudev.h
+++ b/udev/lib/libudev.h
@@ -62,7 +62,6 @@ extern struct udev_device *udev_device_new_from_syspath(struct udev *udev, const
extern struct udev_device *udev_device_new_from_devnum(struct udev *udev, char type, dev_t devnum);
extern struct udev_device *udev_device_new_from_subsystem_sysname(struct udev *udev, const char *subsystem, const char *sysname);
extern struct udev_device *udev_device_get_parent(struct udev_device *udev_device);
-extern struct udev_device *udev_device_get_parent_with_subsystem(struct udev_device *udev_device, const char *subsystem);
extern struct udev_device *udev_device_get_parent_with_subsytem_devtype(struct udev_device *udev_device, const char *subsystem, const char *devtype);
extern struct udev_device *udev_device_ref(struct udev_device *udev_device);
extern void udev_device_unref(struct udev_device *udev_device);
--
1.5.6.3
