[PATCH] Adding disassociate reason as property in dbus (disassoc_bssid as string and disassoc_reason as integer )

Avichal Agarwal avichal.a at samsung.com
Wed Jun 17 01:11:48 EDT 2015


Thanks Dan 
I will made necessary changes and reply soon. :)

Regards
Avichal Agarwal

------- Original Message -------
Sender : Dan Williams<dcbw at redhat.com>
Date : Jun 16, 2015 20:57 (GMT+05:30)
Title : Re: [PATCH] Adding disassociate reason as property in dbus (disassoc_bssid as string and disassoc_reason as integer )

On Mon, 2015-06-15 at 09:25 +0000, Avichal Agarwal wrote:
> From: Avichal Agarwal 
> Date: Mon, 15 Jun 2015 14:41:33 +0530
> Subject: [PATCH] Adding disassociate reason as property in dbus for both wlan
>  and p2p2. It gives disassoc_bssid as string and disassoc_reason as integer.
>  This patch is modified according  to the suggestions given by Dan Williams.
> 
> 
> Signed-off-by: Avichal Agarwal 
> ---
>  wpa_supplicant/dbus/dbus_new.c          |   30 +++++++++++++++++++++++---
>  wpa_supplicant/dbus/dbus_new.h          |    2 ++
>  wpa_supplicant/dbus/dbus_new_handlers.c |   36 +++++++++++++++++++++++++++++++
>  wpa_supplicant/dbus/dbus_new_handlers.h |    4 ++++
>  wpa_supplicant/events.c                 |   10 +++++++--
>  wpa_supplicant/notify.c                 |    9 ++++++++
>  wpa_supplicant/notify.h                 |    1 +
>  wpa_supplicant/wpa_supplicant_i.h       |    4 ++++
>  8 files changed, 91 insertions(+), 5 deletions(-)
> 
> diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c
> index 0696666..ca1a99e 100644
> --- a/wpa_supplicant/dbus/dbus_new.c
> +++ b/wpa_supplicant/dbus/dbus_new.c
> @@ -1893,15 +1893,31 @@ void wpas_dbus_signal_prop_changed(struct wpa_supplicant *wpa_s,
>   prop = "DisconnectReason";
>   flush = TRUE;
>   break;
> + case WPAS_DBUS_PROP_DISASSOCIATE_REASON:
> + prop = "DisassociateReason";
> + flush = TRUE;
> + break;
> + case WPAS_P2P_DBUS_PROP_DISASSOCIATE_REASON:

Actually, we don't need this second P2P property, since a p2p device has
all the non-p2p properties already via the normal D-Bus interface.  The
DisassociateReason property will still be indicated on normal D-Bus
interface even for a P2P-type device, and applications can listen for
that.

D-Bus interfaces are like Java interfaces: a single object can implement
many of them.  In this case, a P2P device object implements at least two
interfaces, WPAS_DBUS_NEW_IFACE_INTERFACE (like all wifi devices) and
WPAS_DBUS_NEW_IFACE_P2PDEVICE.  Applications are already listening to
signals from the object anyway, and can listen for the normal
DisassociateReason property change signal.

> + prop = "DisassociateReasonP2P";
> + flush = TRUE;
> + break;
>   default:
>   wpa_printf(MSG_ERROR, "dbus: %s: Unknown Property value %d",
>      __func__, property);
>   return;
>   }
>  
> - wpa_dbus_mark_property_changed(wpa_s->global->dbus,
> -        wpa_s->dbus_new_path,
> -        WPAS_DBUS_NEW_IFACE_INTERFACE, prop);
> + if(wpa_s->p2p_mgmt) {
> +     wpa_dbus_mark_property_changed(wpa_s->global->dbus,
> +      wpa_s->dbus_new_path,
> +      WPAS_DBUS_NEW_IFACE_P2PDEVICE, prop);

I think this would break notification of normal interface properties on
a p2p interface; they would now all be indicated on the p2p dbus
interface, which breaks the API backwards compatibility.

> + }
> + else {
> + wpa_dbus_mark_property_changed(wpa_s->global->dbus,
> +     wpa_s->dbus_new_path,
> +     WPAS_DBUS_NEW_IFACE_INTERFACE, prop);
> + }
> +
>   if (flush) {
>   wpa_dbus_flush_object_changed_properties(
>   wpa_s->global->dbus->con, wpa_s->dbus_new_path);
> @@ -2997,11 +3013,19 @@ static const struct wpa_dbus_property_desc wpas_dbus_interface_properties[] = {
>     wpas_dbus_getter_persistent_groups,
>     NULL
>   },
> + { "DisassociateReason", WPAS_DBUS_NEW_IFACE_INTERFACE, "si",
> +   wpas_dbus_getter_disassociate_reason,
> +   NULL

As above, I don't believe we need this its redundant with the normal
property that you've added below.

> + },
>  #endif /* CONFIG_P2P */
>   { "DisconnectReason", WPAS_DBUS_NEW_IFACE_INTERFACE, "i",
>     wpas_dbus_getter_disconnect_reason,
>     NULL
>   },
> + { "DisassociateReason", WPAS_DBUS_NEW_IFACE_INTERFACE, "si",
> +   wpas_dbus_getter_disassociate_reason,
> +   NULL
> + },
>   { NULL, NULL, NULL, NULL, NULL }
>  };
>  
> diff --git a/wpa_supplicant/dbus/dbus_new.h b/wpa_supplicant/dbus/dbus_new.h
> index 7503348..b9bdd0a 100644
> --- a/wpa_supplicant/dbus/dbus_new.h
> +++ b/wpa_supplicant/dbus/dbus_new.h
> @@ -29,6 +29,8 @@ enum wpas_dbus_prop {
>   WPAS_DBUS_PROP_CURRENT_AUTH_MODE,
>   WPAS_DBUS_PROP_BSSS,
>   WPAS_DBUS_PROP_DISCONNECT_REASON,
> + WPAS_DBUS_PROP_DISASSOCIATE_REASON,
> + WPAS_P2P_DBUS_PROP_DISASSOCIATE_REASON,
>  };
>  
>  enum wpas_dbus_bss_prop {
> diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c
> index 2a7e2cf..ff3682a 100644
> --- a/wpa_supplicant/dbus/dbus_new_handlers.c
> +++ b/wpa_supplicant/dbus/dbus_new_handlers.c
> @@ -2797,6 +2797,42 @@ dbus_bool_t wpas_dbus_getter_disconnect_reason(DBusMessageIter *iter,
>  
> 
>  /**
> +*  wpas_dbus_getter_disassociate_reason - Get bssid(string) with status code(int) for ASSOC_REJECT
> +* @iter: Pointer to incoming dbus message iter
> +* @error: Location to store error on failure
> +* @user_data: Function specific data
> +* Returns: TRUE on success, FALSE on failure
> +*
> +* Getter for "DisassociateReason" property.
> +*/
> +dbus_bool_t wpas_dbus_getter_disassociate_reason(DBusMessageIter *iter,
> + DBusError *error,
> + void *user_data)
> +{
> + struct wpa_supplicant *wpa_s = user_data;
> + DBusMessageIter variant_iter;
> + DBusMessageIter struct_iter;
> +
> + if (!dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT,
> +   "(si)", &variant_iter) ||
> + !dbus_message_iter_open_container(iter, DBUS_TYPE_STRUCT,
> +   NULL, &struct_iter) ||
> + !dbus_message_iter_append_basic(&struct_iter, DBUS_TYPE_INT32,
> + &wpa_s->disassoc_reason) ||
> + !dbus_message_iter_append_basic(&struct_iter, DBUS_TYPE_STRING,
> + &wpa_s->disassoc_bssid) ||
> + !dbus_message_iter_close_container(iter, &struct_iter) ||
> + !dbus_message_iter_close_container(iter, &variant_iter)) {
> + dbus_set_error(error, DBUS_ERROR_FAILED,

The whitespace is wrong here...  it should be:

+ if (!dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT,
+                                       "(si)", &variant_iter) ||
+     !dbus_message_iter_open_container(iter, DBUS_TYPE_STRUCT,
+                                       NULL, &struct_iter) ||
+     !dbus_message_iter_append_basic(&struct_iter, DBUS_TYPE_INT32,
+                                     &wpa_s->disassoc_reason) ||
+     !dbus_message_iter_append_basic(&struct_iter, DBUS_TYPE_STRING,
+                                     &wpa_s->disassoc_bssid) ||
+     !dbus_message_iter_close_container(iter, &struct_iter) ||
+     !dbus_message_iter_close_container(iter, &variant_iter)) {
+ dbus_set_error(error, DBUS_ERROR_FAILED,

eg, each top-level condition should be left-aligned, and all the
continuing arguments for each function should be aligned with that
function's opening (.  This makes the code flow a lot clearer.

One more rework of this patch and hopefully it'll be done... thanks!

Dan

> +    "%s: error constructing reply variant", __func__);
> + return FALSE;
> + }
> +
> + return TRUE;
> +}
> +
> +
> +/**
>   * wpas_dbus_getter_bss_expire_age - Get BSS entry expiration age
>   * @iter: Pointer to incoming dbus message iter
>   * @error: Location to store error on failure
> diff --git a/wpa_supplicant/dbus/dbus_new_handlers.h b/wpa_supplicant/dbus/dbus_new_handlers.h
> index 50f72ec..bdf74cf 100644
> --- a/wpa_supplicant/dbus/dbus_new_handlers.h
> +++ b/wpa_supplicant/dbus/dbus_new_handlers.h
> @@ -173,6 +173,10 @@ dbus_bool_t wpas_dbus_getter_disconnect_reason(DBusMessageIter *iter,
>          DBusError *error,
>          void *user_data);
>  
> +dbus_bool_t wpas_dbus_getter_disassociate_reason(DBusMessageIter *iter,
> + DBusError *error,
> + void *user_data);
> +
>  dbus_bool_t wpas_dbus_getter_bss_expire_age(DBusMessageIter *iter,
>       DBusError *error, void *user_data);
>  
> diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
> index b615fbd..6a2debe 100644
> --- a/wpa_supplicant/events.c
> +++ b/wpa_supplicant/events.c
> @@ -3295,15 +3295,21 @@ void wpa_supplicant_event(void *ctx, enum wpa_event_type event,
>   break;
>  #endif /* CONFIG_IBSS_RSN */
>   case EVENT_ASSOC_REJECT:
> - if (data->assoc_reject.bssid)
> + wpa_s->disassoc_reason = data->assoc_reject.status_code;
> + os_memset(wpa_s->disassoc_bssid ,0,20);
> + if (data->assoc_reject.bssid) {
> + os_snprintf(wpa_s->disassoc_bssid,20,MACSTR,MAC2STR(data->assoc_reject.bssid));
>   wpa_msg(wpa_s, MSG_INFO, WPA_EVENT_ASSOC_REJECT
>   "bssid=" MACSTR " status_code=%u",
>   MAC2STR(data->assoc_reject.bssid),
>   data->assoc_reject.status_code);
> - else
> + }
> + else {
>   wpa_msg(wpa_s, MSG_INFO, WPA_EVENT_ASSOC_REJECT
>   "status_code=%u",
>   data->assoc_reject.status_code);
> + }
> + wpas_notify_diassociate_reason(wpa_s);
>   if (wpa_s->drv_flags & WPA_DRIVER_FLAGS_SME)
>   sme_event_assoc_reject(wpa_s, data);
>   else {
> diff --git a/wpa_supplicant/notify.c b/wpa_supplicant/notify.c
> index 822db74..32101cb 100644
> --- a/wpa_supplicant/notify.c
> +++ b/wpa_supplicant/notify.c
> @@ -117,6 +117,15 @@ void wpas_notify_disconnect_reason(struct wpa_supplicant *wpa_s)
>  }
>  
> 
> +void wpas_notify_diassociate_reason(struct wpa_supplicant *wpa_s)
> +{
> + if (wpa_s->p2p_mgmt)
> + wpas_dbus_signal_prop_changed(wpa_s, WPAS_P2P_DBUS_PROP_DISASSOCIATE_REASON);
> + else
> + wpas_dbus_signal_prop_changed(wpa_s, WPAS_DBUS_PROP_DISASSOCIATE_REASON);
> +}
> +
> +
>  void wpas_notify_network_changed(struct wpa_supplicant *wpa_s)
>  {
>   if (wpa_s->p2p_mgmt)
> diff --git a/wpa_supplicant/notify.h b/wpa_supplicant/notify.h
> index 1aeec47..f7afe35 100644
> --- a/wpa_supplicant/notify.h
> +++ b/wpa_supplicant/notify.h
> @@ -23,6 +23,7 @@ void wpas_notify_state_changed(struct wpa_supplicant *wpa_s,
>          enum wpa_states new_state,
>          enum wpa_states old_state);
>  void wpas_notify_disconnect_reason(struct wpa_supplicant *wpa_s);
> +void wpas_notify_diassociate_reason(struct wpa_supplicant *wpa_s);
>  void wpas_notify_network_changed(struct wpa_supplicant *wpa_s);
>  void wpas_notify_ap_scan_changed(struct wpa_supplicant *wpa_s);
>  void wpas_notify_bssid_changed(struct wpa_supplicant *wpa_s);
> diff --git a/wpa_supplicant/wpa_supplicant_i.h b/wpa_supplicant/wpa_supplicant_i.h
> index f39fe23..222aa36 100644
> --- a/wpa_supplicant/wpa_supplicant_i.h
> +++ b/wpa_supplicant/wpa_supplicant_i.h
> @@ -909,6 +909,10 @@ struct wpa_supplicant {
>   /* WLAN_REASON_* reason codes. Negative if locally generated. */
>   int disconnect_reason;
>  
> + /*ASSOC_REJECT status code and bssid*/
> + char disassoc_bssid [20];
> + int  disassoc_reason ;
> +
>   struct ext_password_data *ext_pw;
>  
>   struct wpabuf *last_gas_resp, *prev_gas_resp;


More information about the HostAP mailing list