Hi,
This is patch v2, so the next one is PATCH v3 in the Subject line.
On Mon, 2015-06-29 at 17:36 +0530, Harish Jenny K N wrote:
> Handle the failure in add_or_replace_bss_to_network by
> removing the entries from hash tables. The failure happens
> in the following case. Failure in create_group or g_try_new0
> call in add_or_replace_bss_to_network function.
>
> ---
> gsupplicant/supplicant.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
> index 38cbad1..cdfdc85 100755
> --- a/gsupplicant/supplicant.c
> +++ b/gsupplicant/supplicant.c
> @@ -1415,7 +1415,7 @@ static char *create_group(struct g_supplicant_bss *bss)
> return g_string_free(str, FALSE);
> }
>
> -static void add_or_replace_bss_to_network(struct g_supplicant_bss *bss)
> +static int add_or_replace_bss_to_network(struct g_supplicant_bss *bss)
> {
> GSupplicantInterface *interface = bss->interface;
> GSupplicantNetwork *network;
> @@ -1425,7 +1425,7 @@ static void add_or_replace_bss_to_network(struct
> g_supplicant_bss *bss)
> SUPPLICANT_DBG("New group created: %s", group);
>
> if (!group)
> - return;
> + return -1;
Please use a suitable error value from /usr/include/*/errno*.h. Here it
is -ENOMEM.
>
> network = g_hash_table_lookup(interface->network_table, group);
> if (network) {
> @@ -1438,7 +1438,7 @@ static void add_or_replace_bss_to_network(struct
> g_supplicant_bss *bss)
> network = g_try_new0(GSupplicantNetwork, 1);
> if (!network) {
> g_free(group);
> - return;
> + return -1;
As above, -ENOMEM.
> }
>
> network->interface = interface;
> @@ -1484,6 +1484,7 @@ done:
> g_hash_table_replace(network->bss_table, bss->path, bss);
>
> g_hash_table_replace(bss_mapping, bss->path, interface);
> + return 0;
An empty line before the return statement makes this look nicer.
> }
>
> static void bss_rates(DBusMessageIter *iter, void *user_data)
> @@ -1885,7 +1886,8 @@ static void
> interface_bss_added_with_keys(DBusMessageIter *iter,
> supplicant_dbus_property_foreach(iter, bss_property, bss);
>
> bss_compute_security(bss);
> - add_or_replace_bss_to_network(bss);
> + if (add_or_replace_bss_to_network(bss) != 0)
Check against < 0 so that it conforms to the rest of the code.
> + SUPPLICANT_DBG("add_or_replace_bss_to_network failed");
> }
>
> static void interface_bss_added_without_keys(DBusMessageIter *iter,
> @@ -1904,7 +1906,8 @@ static void
> interface_bss_added_without_keys(DBusMessageIter *iter,
> bss_property, bss, NULL);
>
> bss_compute_security(bss);
> - add_or_replace_bss_to_network(bss);
> + if (add_or_replace_bss_to_network(bss) != 0)
Same here.
> + SUPPLICANT_DBG("add_or_replace_bss_to_network failed");
> }
>
> static void update_signal(gpointer key, gpointer value,
> @@ -2485,7 +2488,14 @@ static void signal_bss_changed(const char *path,
> DBusMessageIter *iter)
>
> g_hash_table_remove(interface->network_table, network->group);
>
> - add_or_replace_bss_to_network(new_bss);
> + if (add_or_replace_bss_to_network(new_bss) != 0) {
Same here.
> + /* Remove entries in hash tables to handle the
> + * failure in add_or_replace_bss_to_network
> + */
> + g_hash_table_remove(bss_mapping, path);
> + g_hash_table_remove(interface->bss_mapping, path);
> + g_hash_table_remove(network->bss_table, path);
> + }
>
> return;
> }
Cheers,
Patrik
_______________________________________________
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman
