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