Hi Daniel,
On Tue, Aug 02, 2011 at 01:32:17PM +0200, Daniel Wagner wrote:
> Hi Samuel,
>
> On 08/01/2011 11:50 AM, Samuel Ortiz wrote:
> > Hi Daniel,
> >
> > On Sat, Jul 30, 2011 at 03:17:47PM +0200, Daniel Wagner wrote:
> >> connman_device_remove_network() might be called with only
> >> 1 remaining network available. In this case the service
> >> will be destroyed by the hash table remove call, therefore
> >> the service pointer is bogus.
> >> ---
> >> src/device.c | 1 +
> >> 1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/src/device.c b/src/device.c
> >> index e5bd84d..f0e5b37 100644
> >> --- a/src/device.c
> >> +++ b/src/device.c
> >> @@ -1119,6 +1119,7 @@ int connman_device_remove_network(struct
> >> connman_device *device,
> >> identifier = connman_network_get_identifier(network);
> >> g_hash_table_remove(device->networks, identifier);
> >>
> >> + service = __connman_service_lookup_by_identifier(identifier);
> > If __connman_service_lookup_from_network() gives us an invalid pointer, then
> > we need to fix it. I think having several ways to got from service to
> > network
> > is just going to be confusing.
>
> Nope, __connman_service_lookup_from_network() returns us a valid
> pointer, but after g_hash_table_remove() has been called the pointer
> might be invalid. For example when there is exactly one network for a
> service, in this case the pointer is invalid but not NULL.
>
> The additional lookup makes sure the pointer is updated.
Ok, I see.
> > Moreover, I find it difficult to match this diff with the changelog. How is
> > the service pointer better with lookup_by_identifier() ?
>
> I agree, but how do you want to look it up? Of course we could add
> something like __connman_service_get_number_of_networks() which is also
> something smelly. Any ideas?
What about this:
diff --git a/src/device.c b/src/device.c
index e5bd84d..a3718a9 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1117,11 +1117,13 @@ int connman_device_remove_network(struct
connman_device
service = __connman_service_lookup_from_network(network);
identifier = connman_network_get_identifier(network);
- g_hash_table_remove(device->networks, identifier);
+ g_hash_table_steal(device->networks, identifier);
if (service != NULL)
__connman_service_reset_from_networks(service,
device->networks)
+ free_network(network);
+
return 0;
}
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
_______________________________________________
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
