Hi Martin,
> I have worked out a patch to fix bug 3445. please review it.
>
> The root cause of the bug is:
> 1. "group" is not set to the "network" created by ConnMan to associate a
> hidden AP, so the corresponding service will not be created, and showed.
> 2. However, you can find that the original hidden services visible. It is
> because that the associating with the hidden AP may force
> WPA_SUPPLICANT/driver active scan(probe/request) with the hidden AP and get
> the hidden name. Consequently, the original hidden services will be showed.
> But the service is not be linked to the connected network created by "Join"
> method. That is the reason why you find that the services state is not
> correct.
>
> 3. Currently, when supplicant find the hidden AP network visible, it will
> update the service using the AP/network. Because device_identifity and group
> issued to identify a service, and same SSID, encrypt and manage mode
> compromise group so if several APs in the same group. When suppplicant get
> these APs, it will update the same service using these networks one by one.
> Consequently, the network linking with the last AP will be used to link with
> service. So the service state is not right and all the operations to the
> service all have issues.
>
> The patch resolves the issues through below method:
> 1. set the group to the network created by ConnMan, so the corresponding
> service has chance to be created.
>
> 2. before updating service with the new network, check whether the original
> network linked with the service are active. If it active, the update will not
> go no. That makes sure the service always has the right network.
> Please review the patch
>
> diff --git a/src/device.c b/src/device.c
> index cd9105d..0c97a9c 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -526,6 +526,22 @@ static DBusMessage *set_property(DBusConnection *conn,
> return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
> }
>
> +static char *build_group(const unsigned char *ssid,
> + unsigned int ssid_len,
> + const char *mode,
> + const char *security)
> +{
> + int i;
> + GString *str = g_string_sized_new((ssid_len * 2) + 24);
> + if (ssid_len > 0 && ssid[0] != '\0')
> + for (i = 0; i < ssid_len; i++)
> + g_string_append_printf(str, "%02x", ssid[i]);
> +
> + g_string_append_printf(str, "_%s_%s", mode, security);
> +
> + return g_string_free(str, FALSE);
> +}
> +
> static DBusMessage *join_network(DBusConnection *conn,
> DBusMessage *msg, void *data)
> {
> @@ -534,7 +550,9 @@ static DBusMessage *join_network(DBusConnection *conn,
> enum connman_network_type type;
> DBusMessageIter iter, array;
> int err, index;
> -
> + unsigned int ssid_size;
> + const char *group, *mode, *security;
> + const void *ssid;
> DBG("conn %p", conn);
>
> if (__connman_security_check_privilege(msg,
> @@ -583,6 +601,13 @@ static DBusMessage *join_network(DBusConnection *conn,
> dbus_message_iter_next(&array);
> }
>
> + ssid = connman_network_get_blob(network, "WiFi.SSID", &ssid_size);
> + security = connman_network_get_string(network, "WiFi.Security");
> + mode = connman_network_get_string(network, "WiFi.Mode");
> + group = build_group(ssid, ssid_size, mode, security);
> + DBG("group:%s", group);
> + connman_network_set_group(network, group);
> +
> index = connman_device_get_index(device);
> connman_network_set_index(network, index);
I re-arranged this a little bit and applied it. Looks good to me.
> diff --git a/src/service.c b/src/service.c
> index aa89d5d..c4c4f30 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -616,6 +616,14 @@ void __connman_service_put(struct connman_service
> *service)
> {
> DBG("service %p", service);
>
> + if (service->state == CONNMAN_SERVICE_STATE_CARRIER ||
> + service->state == CONNMAN_SERVICE_STATE_ASSOCIATION ||
> + service->state == CONNMAN_SERVICE_STATE_READY ||
> + service->state == CONNMAN_SERVICE_STATE_CONFIGURATION) {
> + DBG("service is active and can not be put!");
> + return;
> + }
> +
> if (g_atomic_int_dec_and_test(&service->refcount) == TRUE) {
> GSequenceIter *iter;
This is wrong for sure. What are you trying to fix.
> @@ -1176,6 +1184,11 @@ struct connman_service
> *__connman_service_create_from_network(struct connman_net
> const char *ident, *group;
> char *name;
>
> + if (__connman_service_lookup_from_network(network) != NULL) {
> + DBG("service exists!");
> + return -EALREADY;
> + }
> +
> ident = __connman_network_get_ident(network);
> if (ident == NULL)
> return NULL;
Applied this as a separate patch with a connman_error() call. And the
return value is a pointer and not an int ;)
Regards
Marcel
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
