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. > 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? cheers, daniel _______________________________________________ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman