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
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
