Re: [PATCH udev 01/15] udevng: simplify logic in check_usb_device

[Lukasz Nowak <lnowak.tyco@gmail.com>]

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

This works fine for me. I have dropped my parent based patch and rebased the setup_telitqmi changes on top of this.

Lukasz


On 25/03/17 16:57, Jonas Bonn wrote:
> This patch simplifies and cleans up the check_usb_device function a bit
> by doing the two following (slightly intertwined) things:
> 
> 1)  The parent "usb_device" is searched for early in this function and this
> device will always have the ID_VENDOR_ID and ID_MODEL_ID properties.
> As such, we can get them from this device and thereby be certain that
> we _always_ have them available.
> 
> 2)  The logic of iterating the vendor_list table is cleaned up.  It's
> easier to follow and won't be any less efficient.
> ---
>  plugins/udevng.c | 37 ++++++++++++-------------------------
>  1 file changed, 12 insertions(+), 25 deletions(-)
> 
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index 2279bbe..ce2bc28 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -1225,9 +1225,12 @@ static void check_usb_device(struct udev_device *device)
>  	if (devname == NULL)
>  		return;
>  
> +	vendor = udev_device_get_property_value(usb_device, "ID_VENDOR_ID");
> +	model = udev_device_get_property_value(usb_device, "ID_MODEL_ID");
> +
>  	driver = udev_device_get_property_value(usb_device, "OFONO_DRIVER");
>  	if (driver == NULL) {
> -		const char *drv, *vid, *pid;
> +		const char *drv;
>  		unsigned int i;
>  
>  		drv = udev_device_get_property_value(device, "ID_USB_DRIVER");
> @@ -1246,40 +1249,24 @@ static void check_usb_device(struct udev_device *device)
>  			}
>  		}
>  
> -		vid = udev_device_get_property_value(device, "ID_VENDOR_ID");
> -		pid = udev_device_get_property_value(device, "ID_MODEL_ID");
>  
> -		DBG("%s [%s:%s]", drv, vid, pid);
> +		DBG("%s [%s:%s]", drv, vendor, model);
>  
>  		for (i = 0; vendor_list[i].driver; i++) {
>  			if (g_str_equal(vendor_list[i].drv, drv) == FALSE)
>  				continue;
>  
> -			if (vendor_list[i].vid == NULL) {
> -				driver = vendor_list[i].driver;
> -				vendor = vid;
> -				model = pid;
> -				continue;
> +			if (vendor_list[i].vid) {
> +				if (!g_str_equal(vendor_list[i].vid, vendor))
> +					continue;
>  			}
>  
> -			if (vid == NULL || pid == NULL)
> -				continue;
> -
> -			if (g_str_equal(vendor_list[i].vid, vid) == TRUE) {
> -				if (vendor_list[i].pid == NULL) {
> -					driver = vendor_list[i].driver;
> -					vendor = vid;
> -					model = pid;
> +			if (vendor_list[i].pid) {
> +				if (!g_str_equal(vendor_list[i].pid, model))
>  					continue;
> -				}
> -
> -				if (g_strcmp0(vendor_list[i].pid, pid) == 0) {
> -					driver = vendor_list[i].driver;
> -					vendor = vid;
> -					model = pid;
> -					break;
> -				}
>  			}
> +
> +			driver = vendor_list[i].driver;
>  		}
>  
>  		if (driver == NULL)
>