Hi Jonas,

On 03/28/2017 08:18 AM, Jonas Bonn wrote:
> Device properties are generally on the device, on the USB interface
> descriptor, or the on the USB device descriptor.
> ---
>  plugins/udevng.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> @@ -1373,6 +1373,11 @@ static void add_device(const char *syspath, const char *devname,
>  	}
>
>  	label = udev_device_get_property_value(device, "OFONO_LABEL");
> +	if (!label) {
> +		label = udev_device_get_property_value(usb_interface,
> +							"OFONO_LABEL");
> +	}
> +

We prefer not to have {} for single expression if/while/do/for blocks

>  	subsystem = udev_device_get_subsystem(device);
>
>  	if (modem->sysattr != NULL)
> @@ -1495,6 +1500,14 @@ static void check_usb_device(struct udev_device *device)
>  	model = udev_device_get_property_value(usb_device, "ID_MODEL_ID");
>
>  	driver = udev_device_get_property_value(usb_device, "OFONO_DRIVER");
> +	if (!driver) {
> +		struct udev_device *usb_interface;
> +		usb_interface = udev_device_get_parent_with_subsystem_devtype(
> +					device, "usb", "usb_interface");
> +		if (usb_interface)
> +			driver = udev_device_get_property_value(
> +					usb_interface, "OFONO_DRIVER");

Might be cleaner written as:
		struct udev_device *usb_interface =
			udev_device_get_parent_with_subsystem_devtype(
					device, "usb", "usb_interface");

		if (usb_interface)
			driver = udev_device_get_property_value(
					usb_interface, "OFONO_DRIVER");

> +	}
>  	if (driver == NULL) {

item M1.  Separate the two if statements

>  		const char *drv;
>  		unsigned int i;
>

Regards,
-Denis