On Thu, 2009-03-05 at 15:15 -0700, Drew Moseley wrote:
> Dan Williams wrote:
> > I looked over the code there again, and it's correct AFAICT.
> > g_slist_delete_link() in the sync_devices() function should be doing
> > what its intended to do; it removes one link from the linked list
> > priv->devices and frees the link. It doesn't free the data of course,
> > but that should get correctly unref-ed right below. And after that, the
> > device should no longer be in priv->devices at all. That doesn't mean
> > there isn't a refcounting bug, but I'm still not quite sure what's
> > causing the invalid access later on...
>
> Hi Dan,
>
> Looks like the issue is that the delete_link call is incorrect since the node
> being passed in is from the copy of the list. The patch below addresses this
> by building the new list by adding the items that exist and then deleting the
> old list.
>
> Thoughts?
Excellent catch. Thanks! Will commit.
Dan
> Drew
>
>
> diff --git a/src/nm-manager.c b/src/nm-manager.c
> index 30660f1..9a8c1e8 100644
> --- a/src/nm-manager.c
> +++ b/src/nm-manager.c
> @@ -1479,12 +1479,11 @@ static void
> sync_devices (NMManager *self)
> {
> NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
> - GSList *devices;
> + GSList *devices = NULL;
> GSList *iter;
>
> /* Remove devices which are no longer known to HAL */
> - devices = g_slist_copy (priv->devices);
> - for (iter = devices; iter; iter = iter->next) {
> + for (iter = priv->devices; iter; iter = iter->next) {
> NMDevice *device = NM_DEVICE (iter->data);
> const char *udi = nm_device_get_udi (device);
>
> @@ -1493,13 +1492,14 @@ sync_devices (NMManager *self)
> nm_device_set_managed (device, TRUE,
> NM_DEVICE_STATE_REASON_NOW_MANAGED);
> else
> nm_device_set_managed (device, FALSE,
> NM_DEVICE_STATE_REASON_NOW_UNMANAGED);
> + devices = g_slist_prepend(devices, device);
> } else {
> - priv->devices = g_slist_delete_link (priv->devices,
> iter);
> remove_one_device (self, device);
> }
> }
>
> - g_slist_free (devices);
> + g_slist_free (priv->devices);
> + priv->devices = devices;
>
> /* Get any new ones */
> nm_hal_manager_query_devices (priv->hal_mgr);
>
_______________________________________________
NetworkManager-list mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/networkmanager-list
