diff mbox

P2P D-Bus: Use Persistent Group instead of Network object

Message ID 1326710467-30788-1-git-send-email-jean-michelx.bachot@intel.com
State Changes Requested
Headers show

Commit Message

Jean-Michel Bachot Jan. 16, 2012, 10:41 a.m. UTC
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(-)

Comments

Jouni Malinen Jan. 30, 2012, 11:24 p.m. UTC | #1
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 mbox

Patch

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;