[PATCH 1/2] Fix Use hashtable to record udev path

[PATCH 1/2] Fix Use hashtable to record udev path

[Zhenhua Zhang]

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

Sometimes, Udev device 'remove' event could not report correct parent
node of current udev_device. Current code replies on the devpath
attached on the parent node to find modem and then remove it.

This fix is to change the way to store the devpath info into a
hashtable. So that we search hashtable to get devpath and remove the
modem.
---
 plugins/udev.c |   59 +++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/plugins/udev.c b/plugins/udev.c
index 964ac65..6850bf9 100644
--- a/plugins/udev.c
+++ b/plugins/udev.c
@@ -36,6 +36,7 @@
 #include <ofono/log.h>
 
 static GSList *modem_list = NULL;
+static GHashTable *devpath_list = NULL;
 
 static struct ofono_modem *find_modem(const char *devpath)
 {
@@ -258,7 +259,7 @@ static void add_modem(struct udev_device *udev_device)
 {
 	struct ofono_modem *modem;
 	struct udev_device *parent;
-	const char *devpath, *driver;
+	const char *devpath, *curpath, *driver;
 
 	parent = udev_device_get_parent(udev_device);
 	if (parent == NULL)
@@ -294,6 +295,12 @@ static void add_modem(struct udev_device *udev_device)
 		modem_list = g_slist_prepend(modem_list, modem);
 	}
 
+	curpath = udev_device_get_devpath(udev_device);
+	if (curpath == NULL)
+		return;
+
+	g_hash_table_insert(devpath_list, g_strdup(curpath), g_strdup(devpath));
+
 	if (g_strcmp0(driver, "mbm") == 0)
 		add_mbm(modem, udev_device);
 	else if (g_strcmp0(driver, "hso") == 0)
@@ -306,30 +313,28 @@ static void add_modem(struct udev_device *udev_device)
 		add_novatel(modem, udev_device);
 }
 
+static gboolean devpath_remove(gpointer key, gpointer value, gpointer user_data)
+{
+	const char *path = value;
+	const char *devpath = user_data;
+
+	if (!g_strcmp0(path, devpath))
+		return TRUE;
+
+	return FALSE;
+}
+
 static void remove_modem(struct udev_device *udev_device)
 {
 	struct ofono_modem *modem;
-	struct udev_device *parent;
-	const char *devpath, *driver = NULL;
+	const char *curpath = udev_device_get_devpath(udev_device);
+	char *devpath, *remove;
 
-	parent = udev_device_get_parent(udev_device);
-	if (parent == NULL)
+	if (curpath == NULL)
 		return;
 
-	driver = get_driver(parent);
-	if (driver == NULL) {
-		parent = udev_device_get_parent(parent);
-		driver = get_driver(parent);
-		if (driver == NULL) {
-			parent = udev_device_get_parent(parent);
-			driver = get_driver(parent);
-			if (driver == NULL)
-				return;
-		}
-	}
-
-	devpath = udev_device_get_devpath(parent);
-	if (devpath == NULL)
+	devpath = g_hash_table_lookup(devpath_list, curpath);
+	if (!devpath)
 		return;
 
 	modem = find_modem(devpath);
@@ -339,6 +344,12 @@ static void remove_modem(struct udev_device *udev_device)
 	modem_list = g_slist_remove(modem_list, modem);
 
 	ofono_modem_remove(modem);
+
+	remove = g_strdup(devpath);
+
+	g_hash_table_foreach_remove(devpath_list, devpath_remove, remove);
+
+	g_free(remove);
 }
 
 static void enumerate_devices(struct udev *context)
@@ -443,6 +454,13 @@ static void udev_start(void)
 
 static int udev_init(void)
 {
+	devpath_list = g_hash_table_new_full(g_str_hash, g_str_equal,
+						g_free, g_free);
+	if (!devpath_list) {
+		ofono_error("Failed to create udev path list");
+		return -ENOMEM;
+	}
+
 	udev_ctx = udev_new();
 	if (udev_ctx == NULL) {
 		ofono_error("Failed to create udev context");
@@ -483,6 +501,9 @@ static void udev_exit(void)
 	g_slist_free(modem_list);
 	modem_list = NULL;
 
+	g_hash_table_destroy(devpath_list);
+	devpath_list = NULL;
+
 	if (udev_ctx == NULL)
 		return;
 
-- 
1.6.3.3

RE: [PATCH 1/2] Fix Use hashtable to record udev path

[Zhang, Zhenhua]

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

Hi,

Zhang, Zhenhua wrote:
> Sometimes, Udev device 'remove' event could not report correct
> parent node of current udev_device. Current code replies on
> the devpath attached on the parent node to find modem and then
> remove it.
> 
> This fix is to change the way to store the devpath info into a
> hashtable. So that we search hashtable to get devpath and
> remove the modem.
> ---

It's a proposed patch to fix Meego bug #395 [1].

oFono relies on udev to report device remove event. The devpath info of the modem is stored in the parent node of tty port. In below case, we try to find this parent node of /dev/ttyACM0. The expected path of node is ".../usb1/1-4", which is modem device itself. On Netbook, however, it returns ".../usb1" sometimes. In such case, we could not find the correct devpath and modem so that modem was never removed.

My idea is to use a hashtable to store the mapping between tty path and modem path. So that we could find the modem path without depending on udev_device_get_parent() in remove_modem.

Wrong log is listed below. The expected path is '/devices/pci0000:00/0000:00:1d.7/usb1/1-4/', but it returns '.../0000:00:1d.7/usb1', which is Linux USB host controller.

Wrong log 1:
	remove_modem() device 0x9223820
	path #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.1/tty/ttyACM0
	path #1 /devices/pci0000:00/0000:00:1d.7/usb1
	path #2 /devices/pci0000:00/0000:00:1d.7/usb1
	path #3 /devices/pci0000:00/0000:00:1d.7
	path #4 /devices/pci0000:00
	remove_modem() device 0x9222628
	path #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.3/tty/ttyACM1
	path #1 /devices/pci0000:00/0000:00:1d.7/usb1
	path #2 /devices/pci0000:00/0000:00:1d.7/usb1
	path #3 /devices/pci0000:00/0000:00:1d.7
	path #4 /devices/pci0000:00
	remove_modem() device 0x92233a8
	path #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.7/net/wwan0
	path #1 /devices/pci0000:00/0000:00:1d.7/usb1
	path #2 /devices/pci0000:00/0000:00:1d.7/usb1
	path #3 /devices/pci0000:00/0000:00:1d.7
	path #4 /devices/pci0000:00
	remove_modem() device 0x9224520
	path #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.9/tty/ttyACM2
	path #1 /devices/pci0000:00/0000:00:1d.7/usb1
	path #2 /devices/pci0000:00/0000:00:1d.7/usb1
	path #3 /devices/pci0000:00/0000:00:1d.7
	path #4 /devices/pci0000:00

Wrong log 2:
	remove_modem() device 0x9226398
	path #0 /1-4:1.1/tty/ttyACM0
	remove_modem() device 0x9226398
	path #0 /1-4:1.3/tty/ttyACM1
	remove_modem() device 0x9226398
	path #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.9/tty/ttyACM2
	path #1 /devices/pci0000:00/0000:00:1d.7/usb1
	path #2 /devices/pci0000:00/0000:00:1d.7/usb1
	path #3 /devices/pci0000:00/0000:00:1d.7
	path #4 /devices/pci0000:00
	remove_modem() device 0x9224880
	path #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.7/net/wwan0
	path #1 /devices/pci0000:00/0000:00:1d.7/usb1
	path #2 /devices/pci0000:00/0000:00:1d.7/usb1
	path #3 /devices/pci0000:00/0000:00:1d.7
	path #4 /devices/pci0000:00

[1] http://bugs.meego.com/show_bug.cgi?id=395

Regards,
Zhenhua


[-- Attachment #2: udev_ofono.txt --]
[-- Type: text/plain, Size: 8443 bytes --]

----- Plug a USB dangle into netbook ----
ofonod[1829]: plugins/udev.c:add_modem() device 0x9222e78 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:add_modem() add path /devices/pci0000:00/0000:00:1d.7/usb1/1-4
ofonod[1829]: src/modem.c:ofono_modem_create() name: 3558620217367190, type: mbm
ofonod[1829]: src/modem.c:set_modem_property() modem 0x92236b8 property Path
ofonod[1829]: src/modem.c:set_modem_property() modem 0x92236b8 property Registered
ofonod[1829]: plugins/udev.c:add_mbm() desc: Dell Wireless 5530 HSPA Mobile Broadband Minicard Modem
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property Registered
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property ModemDevice
ofonod[1829]: src/modem.c:set_modem_property() modem 0x92236b8 property ModemDevice
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property ModemDevice
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property NetworkInterface
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property Path
ofonod[1829]: plugins/udev.c:add_mbm() desc: Dell Wireless 5530 HSPA Mobile Broadband Minicard NetworkAdapter
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property Registered
ofonod[1829]: src/modem.c:set_modem_property() modem 0x92236b8 property NetworkInterface
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property ModemDevice
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property NetworkInterface
ofonod[1829]: src/modem.c:set_modem_property() modem 0x92236b8 property Registered
ofonod[1829]: src/modem.c:unregister_property() property 0x9223d10
ofonod[1829]: plugins/mbm.c:mbm_probe() 0x92236b8
ofonod[1829]: plugins/udev.c:add_mbm() 0x9222e78 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.7/net/wwan0
ofonod[1829]: plugins/udev.c:add_mbm() parent path /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.7
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property Path
ofonod[1829]: plugins/udev.c:add_mbm() desc: Dell Wireless 5530 HSPA Mobile Broadband Minicard GPS Port
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property Registered
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property Path
ofonod[1829]: plugins/udev.c:add_mbm() desc: Dell Wireless 5530 HSPA Mobile Broadband Minicard Modem 2
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property Registered
ofonod[1829]: plugins/mbm.c:mbm_enable() 0x92236b8
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property ModemDevice
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property DataDevice
ofonod[1829]: plugins/mbm.c:mbm_enable() /dev/ttyACM0, (null)

----- Unplug the USB dangle out, #1 is xxx/usb1, expected path is xxx/usb1/1-4 ----

ofonod[1829]: plugins/udev.c:remove_modem() device 0x9223820 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.1/tty/ttyACM0
ofonod[1829]: plugins/udev.c:print_path() #1 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #2 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #3 /devices/pci0000:00/0000:00:1d.7
ofonod[1829]: plugins/udev.c:print_path() #4 /devices/pci0000:00
ofonod[1829]: plugins/udev.c:remove_modem() device 0x9222628 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.3/tty/ttyACM1
ofonod[1829]: plugins/udev.c:print_path() #1 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #2 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #3 /devices/pci0000:00/0000:00:1d.7
ofonod[1829]: plugins/udev.c:print_path() #4 /devices/pci0000:00
ofonod[1829]: plugins/udev.c:remove_modem() device 0x92233a8 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.7/net/wwan0
ofonod[1829]: plugins/udev.c:print_path() #1 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #2 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #3 /devices/pci0000:00/0000:00:1d.7
ofonod[1829]: plugins/udev.c:print_path() #4 /devices/pci0000:00
ofonod[1829]: plugins/udev.c:remove_modem() device 0x9224520 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.9/tty/ttyACM2
ofonod[1829]: plugins/udev.c:print_path() #1 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #2 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #3 /devices/pci0000:00/0000:00:1d.7
ofonod[1829]: plugins/udev.c:print_path() #4 /devices/pci0000:00

----- Unplug the USB dangle out, sometime the path missed usb path prefix ----

ofonod[1829]: plugins/udev.c:remove_modem() device 0x9226398 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /1-4:1.1/tty/ttyACM0
ofonod[1829]: plugins/udev.c:remove_modem() device 0x9226398 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /1-4:1.3/tty/ttyACM1
ofonod[1829]: plugins/udev.c:remove_modem() device 0x9226398 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.9/tty/ttyACM2
ofonod[1829]: plugins/udev.c:print_path() #1 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #2 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #3 /devices/pci0000:00/0000:00:1d.7
ofonod[1829]: plugins/udev.c:print_path() #4 /devices/pci0000:00
ofonod[1829]: plugins/udev.c:remove_modem() device 0x9224880 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.7/net/wwan0
ofonod[1829]: plugins/udev.c:print_path() #1 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #2 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #3 /devices/pci0000:00/0000:00:1d.7
ofonod[1829]: plugins/udev.c:print_path() #4 /devices/pci0000:00



----- Unplug the USB dangle out, expected result ----

ofonod[1829]: plugins/udev.c:remove_modem() device 0x92237e0 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.1/tty/ttyACM0
ofonod[1829]: plugins/udev.c:print_path() #1 /devices/pci0000:00/0000:00:1d.7/usb1/1-4
ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property Path
ofonod[1829]: src/modem.c:ofono_modem_remove() 0x92236b8
ofonod[1829]: plugins/mbm.c:mbm_remove() 0x92236b8
ofonod[1829]: src/modem.c:unregister_property() property 0x9223db8
ofonod[1829]: src/modem.c:unregister_property() property 0x9223640
ofonod[1829]: src/modem.c:unregister_property() property 0x9223678
ofonod[1829]: src/modem.c:unregister_property() property 0x9223798
ofonod[1829]: plugins/udev.c:remove_modem() device 0x9223578 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.3/tty/ttyACM1
ofonod[1829]: plugins/udev.c:print_path() #1 /devices/pci0000:00/0000:00:1d.7/usb1/1-4
ofonod[1829]: plugins/udev.c:print_path() #2 /devices/pci0000:00/0000:00:1d.7/usb1/1-4
ofonod[1829]: plugins/udev.c:print_path() #3 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #4 /devices/pci0000:00/0000:00:1d.7
ofonod[1829]: plugins/udev.c:remove_modem() device 0x92236b8 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.9/tty/ttyACM2
ofonod[1829]: plugins/udev.c:print_path() #1 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #2 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #3 /devices/pci0000:00/0000:00:1d.7
ofonod[1829]: plugins/udev.c:print_path() #4 /devices/pci0000:00
ofonod[1829]: plugins/udev.c:remove_modem() device 0x92231c8 -> udev 0x9220ec8
ofonod[1829]: plugins/udev.c:print_path() #0 /devices/pci0000:00/0000:00:1d.7/usb1/1-4/1-4:1.7/net/wwan0
ofonod[1829]: plugins/udev.c:print_path() #1 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #2 /devices/pci0000:00/0000:00:1d.7/usb1
ofonod[1829]: plugins/udev.c:print_path() #3 /devices/pci0000:00/0000:00:1d.7
ofonod[1829]: plugins/udev.c:print_path() #4 /devices/pci0000:00

Re: [PATCH 1/2] Fix Use hashtable to record udev path

[Denis Kenzior]

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

Hi Zhenhua,

> Sometimes, Udev device 'remove' event could not report correct parent
> node of current udev_device. Current code replies on the devpath
> attached on the parent node to find modem and then remove it.
> 
> This fix is to change the way to store the devpath info into a
> hashtable. So that we search hashtable to get devpath and remove the
> modem.
> ---
>  plugins/udev.c |   59
>  +++++++++++++++++++++++++++++++++++++------------------ 1 files changed,
>  40 insertions(+), 19 deletions(-)
> 
> diff --git a/plugins/udev.c b/plugins/udev.c
> index 964ac65..6850bf9 100644
> --- a/plugins/udev.c
> +++ b/plugins/udev.c
> @@ -36,6 +36,7 @@
>  #include <ofono/log.h>
> 
>  static GSList *modem_list = NULL;
> +static GHashTable *devpath_list = NULL;
> 
>  static struct ofono_modem *find_modem(const char *devpath)
>  {
> @@ -258,7 +259,7 @@ static void add_modem(struct udev_device *udev_device)
>  {
>  	struct ofono_modem *modem;
>  	struct udev_device *parent;
> -	const char *devpath, *driver;
> +	const char *devpath, *curpath, *driver;
> 
>  	parent = udev_device_get_parent(udev_device);
>  	if (parent == NULL)
> @@ -294,6 +295,12 @@ static void add_modem(struct udev_device *udev_device)
>  		modem_list = g_slist_prepend(modem_list, modem);
>  	}
> 
> +	curpath = udev_device_get_devpath(udev_device);
> +	if (curpath == NULL)
> +		return;
> +
> +	g_hash_table_insert(devpath_list, g_strdup(curpath), g_strdup(devpath));
> +
>  	if (g_strcmp0(driver, "mbm") == 0)
>  		add_mbm(modem, udev_device);
>  	else if (g_strcmp0(driver, "hso") == 0)
> @@ -306,30 +313,28 @@ static void add_modem(struct udev_device
>  *udev_device) add_novatel(modem, udev_device);
>  }
> 
> +static gboolean devpath_remove(gpointer key, gpointer value, gpointer
>  user_data) +{
> +	const char *path = value;
> +	const char *devpath = user_data;
> +
> +	if (!g_strcmp0(path, devpath))
> +		return TRUE;

How about a simple return g_str_equals here?

> +
> +	return FALSE;
> +}
> +
>  static void remove_modem(struct udev_device *udev_device)
>  {
>  	struct ofono_modem *modem;
> -	struct udev_device *parent;
> -	const char *devpath, *driver = NULL;
> +	const char *curpath = udev_device_get_devpath(udev_device);
> +	char *devpath, *remove;
> 
> -	parent = udev_device_get_parent(udev_device);
> -	if (parent == NULL)
> +	if (curpath == NULL)
>  		return;
> 
> -	driver = get_driver(parent);
> -	if (driver == NULL) {
> -		parent = udev_device_get_parent(parent);
> -		driver = get_driver(parent);
> -		if (driver == NULL) {
> -			parent = udev_device_get_parent(parent);
> -			driver = get_driver(parent);
> -			if (driver == NULL)
> -				return;
> -		}
> -	}
> -
> -	devpath = udev_device_get_devpath(parent);
> -	if (devpath == NULL)
> +	devpath = g_hash_table_lookup(devpath_list, curpath);
> +	if (!devpath)
>  		return;
> 
>  	modem = find_modem(devpath);
> @@ -339,6 +344,12 @@ static void remove_modem(struct udev_device
>  *udev_device) modem_list = g_slist_remove(modem_list, modem);
> 
>  	ofono_modem_remove(modem);
> +
> +	remove = g_strdup(devpath);
> +
> +	g_hash_table_foreach_remove(devpath_list, devpath_remove, remove);
> +
> +	g_free(remove);
>  }
> 
>  static void enumerate_devices(struct udev *context)
> @@ -443,6 +454,13 @@ static void udev_start(void)
> 
>  static int udev_init(void)
>  {
> +	devpath_list = g_hash_table_new_full(g_str_hash, g_str_equal,
> +						g_free, g_free);
> +	if (!devpath_list) {
> +		ofono_error("Failed to create udev path list");
> +		return -ENOMEM;
> +	}
> +

You now need to take care of freeing the devpath_list on any further error 
conditions that might occur, otherwise you leak memory.

>  	udev_ctx = udev_new();
>  	if (udev_ctx == NULL) {
>  		ofono_error("Failed to create udev context");
> @@ -483,6 +501,9 @@ static void udev_exit(void)
>  	g_slist_free(modem_list);
>  	modem_list = NULL;
> 
> +	g_hash_table_destroy(devpath_list);
> +	devpath_list = NULL;
> +
>  	if (udev_ctx == NULL)
>  		return;
> 

Regards,
-Denis

[PATCH 1/2] Fix Use hashtable to record udev path

[Zhenhua Zhang]

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

Sometimes, Udev device 'remove' event could not report correct parent
node of current udev_device. Current code replies on the devpath
attached on the parent node to find modem and then remove it.

This fix is to change the way to store the devpath info into a
hashtable. So that we search hashtable to get devpath and remove the
modem.
---
 plugins/udev.c |   58 +++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/plugins/udev.c b/plugins/udev.c
index 4aaeeb9..3a6ea28 100644
--- a/plugins/udev.c
+++ b/plugins/udev.c
@@ -36,6 +36,7 @@
 #include <ofono/log.h>
 
 static GSList *modem_list = NULL;
+static GHashTable *devpath_list = NULL;
 
 static struct ofono_modem *find_modem(const char *devpath)
 {
@@ -259,7 +260,7 @@ static void add_modem(struct udev_device *udev_device)
 {
 	struct ofono_modem *modem;
 	struct udev_device *parent;
-	const char *devpath, *driver;
+	const char *devpath, *curpath, *driver;
 
 	parent = udev_device_get_parent(udev_device);
 	if (parent == NULL)
@@ -295,6 +296,12 @@ static void add_modem(struct udev_device *udev_device)
 		modem_list = g_slist_prepend(modem_list, modem);
 	}
 
+	curpath = udev_device_get_devpath(udev_device);
+	if (curpath == NULL)
+		return;
+
+	g_hash_table_insert(devpath_list, g_strdup(curpath), g_strdup(devpath));
+
 	if (g_strcmp0(driver, "mbm") == 0)
 		add_mbm(modem, udev_device);
 	else if (g_strcmp0(driver, "hso") == 0)
@@ -307,30 +314,25 @@ static void add_modem(struct udev_device *udev_device)
 		add_novatel(modem, udev_device);
 }
 
+static gboolean devpath_remove(gpointer key, gpointer value, gpointer user_data)
+{
+	const char *path = value;
+	const char *devpath = user_data;
+
+	return g_str_equal(path, devpath);
+}
+
 static void remove_modem(struct udev_device *udev_device)
 {
 	struct ofono_modem *modem;
-	struct udev_device *parent;
-	const char *devpath, *driver = NULL;
+	const char *curpath = udev_device_get_devpath(udev_device);
+	char *devpath, *remove;
 
-	parent = udev_device_get_parent(udev_device);
-	if (parent == NULL)
+	if (curpath == NULL)
 		return;
 
-	driver = get_driver(parent);
-	if (driver == NULL) {
-		parent = udev_device_get_parent(parent);
-		driver = get_driver(parent);
-		if (driver == NULL) {
-			parent = udev_device_get_parent(parent);
-			driver = get_driver(parent);
-			if (driver == NULL)
-				return;
-		}
-	}
-
-	devpath = udev_device_get_devpath(parent);
-	if (devpath == NULL)
+	devpath = g_hash_table_lookup(devpath_list, curpath);
+	if (!devpath)
 		return;
 
 	modem = find_modem(devpath);
@@ -340,6 +342,12 @@ static void remove_modem(struct udev_device *udev_device)
 	modem_list = g_slist_remove(modem_list, modem);
 
 	ofono_modem_remove(modem);
+
+	remove = g_strdup(devpath);
+
+	g_hash_table_foreach_remove(devpath_list, devpath_remove, remove);
+
+	g_free(remove);
 }
 
 static void enumerate_devices(struct udev *context)
@@ -444,15 +452,24 @@ static void udev_start(void)
 
 static int udev_init(void)
 {
+	devpath_list = g_hash_table_new_full(g_str_hash, g_str_equal,
+						g_free, g_free);
+	if (!devpath_list) {
+		ofono_error("Failed to create udev path list");
+		return -ENOMEM;
+	}
+
 	udev_ctx = udev_new();
 	if (udev_ctx == NULL) {
 		ofono_error("Failed to create udev context");
+		g_hash_table_destroy(devpath_list);
 		return -EIO;
 	}
 
 	udev_mon = udev_monitor_new_from_netlink(udev_ctx, "udev");
 	if (udev_mon == NULL) {
 		ofono_error("Failed to create udev monitor");
+		g_hash_table_destroy(devpath_list);
 		udev_unref(udev_ctx);
 		udev_ctx = NULL;
 		return -EIO;
@@ -484,6 +501,9 @@ static void udev_exit(void)
 	g_slist_free(modem_list);
 	modem_list = NULL;
 
+	g_hash_table_destroy(devpath_list);
+	devpath_list = NULL;
+
 	if (udev_ctx == NULL)
 		return;
 
-- 
1.6.3.3

[PATCH 2/2] Fix update driver attached status by CGREG notify

[Zhenhua Zhang]

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

This fix is for Meego bug #389. Dell 5530 modem reports 'ERROR' when
'AT+CGATT=0'. In the error case, we need to wait until the context
deactivation returns and then check netreg status. If status is zero,
we update driver_attached to FALSE.
---
 src/gprs.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/src/gprs.c b/src/gprs.c
index dfb6d16..8e065d2 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -1325,6 +1325,14 @@ void ofono_gprs_status_notify(struct ofono_gprs *gprs, int status)
 	if (gprs->driver_attached == FALSE)
 		return;
 
+	/* Dell 5530 reports 'ERROR' when we try to detach by 'AT+CGATT=0'
+	 * immediately after deactivating the context. So we wait until
+	 * the context deactivation returns and then set driver_attached to
+	 * FALSE when the netreg status is not registered & not searching
+	 */
+	if (status == NETWORK_REGISTRATION_STATUS_NOT_REGISTERED)
+		gprs->driver_attached = FALSE;
+
 	gprs->status = status;
 	gprs_attached_update(gprs);
 }
-- 
1.6.3.3

Re: [PATCH 1/2] Fix Use hashtable to record udev path

[Denis Kenzior]

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

Hi Zhenhua,

> Sometimes, Udev device 'remove' event could not report correct parent
> node of current udev_device. Current code replies on the devpath
> attached on the parent node to find modem and then remove it.
> 
> This fix is to change the way to store the devpath info into a
> hashtable. So that we search hashtable to get devpath and remove the
> modem.

Patch has been applied, thanks.

Regards,
-Denis