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

[Jouni Malinen]

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.

-- 
Jouni Malinen                                            PGP id EFC895FA