Re: [PATCH 2/3] Udevng: Improve modem properties detection

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2888 bytes --]

Hi Martin,

On 05/02/2016 11:33 AM, Martin Chaplet wrote:
>   * Add driver's subsystem information (net, usb, tty, ...)
>   * Improve interface number extraction by scanning also device and
>   parent attributes
>
> Signed-off-by: Martin Chaplet <m.chaplet@kerlink.fr>

No signed-off-by please.  See HACKING, 'Submitting Patches' section.

> ---
>   plugins/udevng.c | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index 9643b0a..8ca4471 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -55,6 +55,7 @@ struct device_info {
>   	char *number;
>   	char *label;
>   	char *sysattr;
> +	char *subsystem;
>   };
>
>   static gboolean setup_isi(struct modem_info *modem)
> @@ -939,6 +940,7 @@ static void destroy_modem(gpointer data)
>   		g_free(info->number);
>   		g_free(info->label);
>   		g_free(info->sysattr);
> +		g_free(info->subsystem);
>   		g_free(info);
>
>   		list->data = NULL;
> @@ -997,9 +999,10 @@ static void add_device(const char *syspath, const char *devname,
>   			const char *model, struct udev_device *device)
>   {
>   	struct udev_device *intf;
> -	const char *devpath, *devnode, *interface, *number, *label, *sysattr;
> +	const char *devpath, *devnode, *interface, *number, *label, *sysattr, *subsystem;
>   	struct modem_info *modem;
>   	struct device_info *info;
> +	struct udev_device *parent;
>
>   	devpath = udev_device_get_syspath(device);
>   	if (devpath == NULL)
> @@ -1037,7 +1040,19 @@ static void add_device(const char *syspath, const char *devname,
>   	interface = udev_device_get_property_value(intf, "INTERFACE");
>   	number = udev_device_get_property_value(device, "ID_USB_INTERFACE_NUM");
>
> +	/* If environment variable is not set, get value from attributes (or parent's ones) */
> +	if(number == NULL)
> +	{

Not our style.  oFono uses the Linux Kernel coding style, so the '{' 
should be on the same line as the if.

> +		number = udev_device_get_sysattr_value(device, "bInterfaceNumber");
> +		if(number == NULL)
> +		{

as above

> +			parent = udev_device_get_parent(device);
> +			number = udev_device_get_sysattr_value(parent, "bInterfaceNumber");
> +		}
> +	}
> +
>   	label = udev_device_get_property_value(device, "OFONO_LABEL");
> +	subsystem = udev_device_get_subsystem(device);
>
>   	if (modem->sysattr != NULL)
>   		sysattr = udev_device_get_sysattr_value(device, modem->sysattr);
> @@ -1059,6 +1074,7 @@ static void add_device(const char *syspath, const char *devname,
>   	info->number = g_strdup(number);
>   	info->label = g_strdup(label);
>   	info->sysattr = g_strdup(sysattr);
> +	info->subsystem = g_strdup(subsystem);
>
>   	modem->devices = g_slist_insert_sorted(modem->devices, info,
>   							compare_device);
>

Regards,
-Denis