Message ID | 1326710467-30788-1-git-send-email-jean-michelx.bachot@intel.com |
---|---|
State | Changes Requested |
Headers | show |
On Mon, Jan 16, 2012 at 11:41:07AM +0100, Jean-Michel Bachot wrote: > Fix the follwing issue in P2P D-Bus API : > When a persistent group is started, wpa_supplicant does not create a > network object but a persistent group object instead. > Hence, emit the network oject wihtin the group started signal > in this case does not make sense and is an error. > Fix is to emit the persistent group object registered instead of > network object in case of persistent group started. Could you please describe this in some more detail? I'm not sure I understand what exactly you mean with starting a persistent group and how this is supposed to show differently in the D-Bus interface. The change itself seemed to change the prefix based on network_is_persistent_group(). That is not really for starting a persistent group, but for storing credentials for a persistent group. Based on a quick check on what D-Feet shows, I did not see a clear difference, so I'm clearly missing something here.. Is this in any way related to how persistent group objects get unregistered? I'm seeing a memory leak issue and D-Bus unregistration issue when killing wpa_supplicant process while a persistent group started with "p2p_group_add persistent=0" was running: dbus: Register interface object '/fi/w1/wpa_supplicant1/Interfaces/1' dbus: Register persistent group object '/fi/w1/wpa_supplicant1/Interfaces/1/PersistentGroups/0' dbus: Register persistent group object '/fi/w1/wpa_supplicant1/Interfaces/1/PersistentGroups/1' dbus: Register persistent group object '/fi/w1/wpa_supplicant1/Interfaces/1/PersistentGroups/2' wlan0: Added interface wlan0 (those are three sets of persistent group credentials from configuration file) RX ctrl_iface - hexdump_ascii(len=26): 50 32 50 5f 47 52 4f 55 50 5f 41 44 44 20 70 65 P2P_GROUP_ADD pe 72 73 69 73 74 65 6e 74 3d 32 rsistent=2 P2P: Update existing persistent group entry dbus: Register group object '/fi/w1/wpa_supplicant1/Interfaces/1/Groups/yc' Ctrl-C --> dbus: Unregister persistent group object '/fi/w1/wpa_supplicant1/Interfaces/1/PersistentGroups/0' dbus: Unregister persistent group object '/fi/w1/wpa_supplicant1/Interfaces/1/PersistentGroups/1' dbus: Unregister persistent group object '/fi/w1/wpa_supplicant1/Interfaces/1/PersistentGroups/2' dbus: Unregister persistent group object '/fi/w1/wpa_supplicant1/Interfaces/1/PersistentGroups/3' dbus: wpa_dbus_unregister_object_per_iface: Could not obtain object's private data: /fi/w1/wpa_supplicant1/Interfaces/1/PersistentGroups/3 Attempted to unregister path (path[0] = fi path[1] = w1) which isn't registered Where did that group 3 come from? Number of memory allocations from dbus_new.c are not freed in this case, so it looks like something did not get freed properly. > diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c > @@ -984,6 +984,8 @@ void wpas_dbus_signal_p2p_group_started(struct wpa_supplicant *wpa_s, > @@ -1020,14 +1022,27 @@ void wpas_dbus_signal_p2p_group_started(struct wpa_supplicant *wpa_s, > + if (network_is_persistent_group((struct wpa_ssid *)ssid)) { > + persistent_group = TRUE; > + os_snprintf(pgrp_obj_path, WPAS_DBUS_OBJECT_PATH_MAX, > + "%s/" WPAS_DBUS_NEW_PERSISTENT_GROUPS_PART "/%u", > + wpa_s->parent->dbus_new_path, network_id); Should this part be within #ifdef CONFIG_P2P like other uses of network_is_persistent_group() in this file? This seemed to build even without P2P support, so this may not be that much of a difference in practice.
diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c index 2bf9be8..6fd86ba 100644 --- a/wpa_supplicant/dbus/dbus_new.c +++ b/wpa_supplicant/dbus/dbus_new.c @@ -984,6 +984,8 @@ void wpas_dbus_signal_p2p_group_started(struct wpa_supplicant *wpa_s, struct wpas_dbus_priv *iface; char net_obj_path[WPAS_DBUS_OBJECT_PATH_MAX]; char group_obj_path[WPAS_DBUS_OBJECT_PATH_MAX]; + char pgrp_obj_path[WPAS_DBUS_OBJECT_PATH_MAX]; + dbus_bool_t persistent_group = FALSE; iface = wpa_s->parent->global->dbus; @@ -1020,14 +1022,27 @@ void wpas_dbus_signal_p2p_group_started(struct wpa_supplicant *wpa_s, client ? "client" : "GO")) goto nomem; - os_snprintf(net_obj_path, WPAS_DBUS_OBJECT_PATH_MAX, - "%s/" WPAS_DBUS_NEW_NETWORKS_PART "/%u", - wpa_s->parent->dbus_new_path, network_id); + /* + * If it is a persistent group register it as such. + * This is to handle cases where an interface is being initialized + * with a list of networks read from config. + */ + if (network_is_persistent_group((struct wpa_ssid *)ssid)) { + persistent_group = TRUE; + os_snprintf(pgrp_obj_path, WPAS_DBUS_OBJECT_PATH_MAX, + "%s/" WPAS_DBUS_NEW_PERSISTENT_GROUPS_PART "/%u", + wpa_s->parent->dbus_new_path, network_id); + } else { + os_snprintf(net_obj_path, WPAS_DBUS_OBJECT_PATH_MAX, + "%s/" WPAS_DBUS_NEW_NETWORKS_PART "/%u", + wpa_s->parent->dbus_new_path, network_id); + } if (!wpa_dbus_dict_append_object_path(&dict_iter, "group_object", group_obj_path) || - !wpa_dbus_dict_append_object_path(&dict_iter, "network_object", - net_obj_path) || + !wpa_dbus_dict_append_object_path(&dict_iter, + persistent_group ? "persistent_group_object" : "network_object", + persistent_group ? pgrp_obj_path : net_obj_path) || !wpa_dbus_dict_close_write(&iter, &dict_iter)) goto nomem;
Fix the follwing issue in P2P D-Bus API : When a persistent group is started, wpa_supplicant does not create a network object but a persistent group object instead. Hence, emit the network oject wihtin the group started signal in this case does not make sense and is an error. Fix is to emit the persistent group object registered instead of network object in case of persistent group started. Signed-off-by: Jean-Michel Bachot <jean-michelx.bachot@intel.com> --- wpa_supplicant/dbus/dbus_new.c | 25 ++++++++++++++++++++----- 1 files changed, 20 insertions(+), 5 deletions(-)