clean-up setting iface->p2p_mgmt flag

Message ID 20180212192906.14583-1-vvavrychuk@gmail.com
State Accepted
Headers show
Series
  • clean-up setting iface->p2p_mgmt flag
Related show

Commit Message

Vasyl Vavrychuk Feb. 12, 2018, 7:29 p.m.
Previously we set this flag to one in wpa_supplicant_init_iface if
Wi-Fi controller does not have dedicated P2P-interface.

This setting had effect only in scope of wpa_supplicant_init_iface and
it contradicts with comment to 'struct wpa_interface > p2p_mgmt' field.
This comment says that this flag is used only if Wi-Fi controller has
dedicated P2P-device interface.

Also it contradicts with usage of similiar p2p_mgmt field in
wpa_supplicant struct. Again field p2p_mgmt in wpa_supplicant struct
is set only for dedicated P2P-device interface.

After this change wpa_interface become input argument to
wpa_supplicant_init_iface that we are not modifying.

Signed-off-by: Vasyl Vavrychuk <vvavrychuk@gmail.com>
---
 wpa_supplicant/wpa_supplicant.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Comments

Vasyl Vavrychuk March 4, 2018, 3 p.m. | #1
Just reminding about this issue.

On Mon, Feb 12, 2018 at 9:29 PM, Vasyl Vavrychuk <vvavrychuk@gmail.com> wrote:
> Previously we set this flag to one in wpa_supplicant_init_iface if
> Wi-Fi controller does not have dedicated P2P-interface.
>
> This setting had effect only in scope of wpa_supplicant_init_iface and
> it contradicts with comment to 'struct wpa_interface > p2p_mgmt' field.
> This comment says that this flag is used only if Wi-Fi controller has
> dedicated P2P-device interface.
>
> Also it contradicts with usage of similiar p2p_mgmt field in
> wpa_supplicant struct. Again field p2p_mgmt in wpa_supplicant struct
> is set only for dedicated P2P-device interface.
>
> After this change wpa_interface become input argument to
> wpa_supplicant_init_iface that we are not modifying.
>
> Signed-off-by: Vasyl Vavrychuk <vvavrychuk@gmail.com>
> ---
>  wpa_supplicant/wpa_supplicant.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
> index fcb267764..73033eb3a 100644
> --- a/wpa_supplicant/wpa_supplicant.c
> +++ b/wpa_supplicant/wpa_supplicant.c
> @@ -5123,7 +5123,7 @@ radio_work_pending(struct wpa_supplicant *wpa_s, const char *type)
>
>
>  static int wpas_init_driver(struct wpa_supplicant *wpa_s,
> -                           struct wpa_interface *iface)
> +                           const struct wpa_interface *iface)
>  {
>         const char *ifname, *driver, *rn;
>
> @@ -5207,7 +5207,7 @@ static void wpas_gas_server_tx(void *ctx, int freq, const u8 *da,
>  #endif /* CONFIG_GAS_SERVER */
>
>  static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
> -                                    struct wpa_interface *iface)
> +                                    const struct wpa_interface *iface)
>  {
>         struct wpa_driver_capa capa;
>         int capa_res;
> @@ -5408,8 +5408,6 @@ static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
>          */
>         if (wpa_s->drv_flags & WPA_DRIVER_FLAGS_DEDICATED_P2P_DEVICE)
>                 wpa_s->p2p_mgmt = iface->p2p_mgmt;
> -       else
> -               iface->p2p_mgmt = 1;
>
>         if (wpa_s->num_multichan_concurrent == 0)
>                 wpa_s->num_multichan_concurrent = 1;
> @@ -5418,11 +5416,10 @@ static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
>                 return -1;
>
>  #ifdef CONFIG_TDLS
> -       if ((!iface->p2p_mgmt ||
> -            !(wpa_s->drv_flags &
> -              WPA_DRIVER_FLAGS_DEDICATED_P2P_DEVICE)) &&
> -           wpa_tdls_init(wpa_s->wpa))
> -               return -1;
> +       if (!iface->p2p_mgmt) {
> +               if (wpa_tdls_init(wpa_s->wpa))
> +                       return -1;
> +       }
>  #endif /* CONFIG_TDLS */
>
>         if (wpa_s->conf->country[0] && wpa_s->conf->country[1] &&
> @@ -5493,9 +5490,12 @@ static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
>                 return -1;
>         }
>
> -       if (iface->p2p_mgmt && wpas_p2p_init(wpa_s->global, wpa_s) < 0) {
> -               wpa_msg(wpa_s, MSG_ERROR, "Failed to init P2P");
> -               return -1;
> +       if (!(wpa_s->drv_flags & WPA_DRIVER_FLAGS_DEDICATED_P2P_DEVICE) ||
> +           wpa_s->p2p_mgmt) {
> +               if (wpas_p2p_init(wpa_s->global, wpa_s) < 0) {
> +                       wpa_msg(wpa_s, MSG_ERROR, "Failed to init P2P");
> +                       return -1;
> +               }
>         }
>
>         if (wpa_bss_init(wpa_s) < 0)
> --
> 2.11.0
>
Jouni Malinen April 2, 2018, 10:32 a.m. | #2
On Mon, Feb 12, 2018 at 09:29:06PM +0200, Vasyl Vavrychuk wrote:
> Previously we set this flag to one in wpa_supplicant_init_iface if
> Wi-Fi controller does not have dedicated P2P-interface.
> 
> This setting had effect only in scope of wpa_supplicant_init_iface and
> it contradicts with comment to 'struct wpa_interface > p2p_mgmt' field.
> This comment says that this flag is used only if Wi-Fi controller has
> dedicated P2P-device interface.
> 
> Also it contradicts with usage of similiar p2p_mgmt field in
> wpa_supplicant struct. Again field p2p_mgmt in wpa_supplicant struct
> is set only for dedicated P2P-device interface.
> 
> After this change wpa_interface become input argument to
> wpa_supplicant_init_iface that we are not modifying.

Thanks, applied.

Patch

diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index fcb267764..73033eb3a 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -5123,7 +5123,7 @@  radio_work_pending(struct wpa_supplicant *wpa_s, const char *type)
 
 
 static int wpas_init_driver(struct wpa_supplicant *wpa_s,
-			    struct wpa_interface *iface)
+			    const struct wpa_interface *iface)
 {
 	const char *ifname, *driver, *rn;
 
@@ -5207,7 +5207,7 @@  static void wpas_gas_server_tx(void *ctx, int freq, const u8 *da,
 #endif /* CONFIG_GAS_SERVER */
 
 static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
-				     struct wpa_interface *iface)
+				     const struct wpa_interface *iface)
 {
 	struct wpa_driver_capa capa;
 	int capa_res;
@@ -5408,8 +5408,6 @@  static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
 	 */
 	if (wpa_s->drv_flags & WPA_DRIVER_FLAGS_DEDICATED_P2P_DEVICE)
 		wpa_s->p2p_mgmt = iface->p2p_mgmt;
-	else
-		iface->p2p_mgmt = 1;
 
 	if (wpa_s->num_multichan_concurrent == 0)
 		wpa_s->num_multichan_concurrent = 1;
@@ -5418,11 +5416,10 @@  static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
 		return -1;
 
 #ifdef CONFIG_TDLS
-	if ((!iface->p2p_mgmt ||
-	     !(wpa_s->drv_flags &
-	       WPA_DRIVER_FLAGS_DEDICATED_P2P_DEVICE)) &&
-	    wpa_tdls_init(wpa_s->wpa))
-		return -1;
+	if (!iface->p2p_mgmt) {
+		if (wpa_tdls_init(wpa_s->wpa))
+			return -1;
+	}
 #endif /* CONFIG_TDLS */
 
 	if (wpa_s->conf->country[0] && wpa_s->conf->country[1] &&
@@ -5493,9 +5490,12 @@  static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
 		return -1;
 	}
 
-	if (iface->p2p_mgmt && wpas_p2p_init(wpa_s->global, wpa_s) < 0) {
-		wpa_msg(wpa_s, MSG_ERROR, "Failed to init P2P");
-		return -1;
+	if (!(wpa_s->drv_flags & WPA_DRIVER_FLAGS_DEDICATED_P2P_DEVICE) ||
+	    wpa_s->p2p_mgmt) {
+		if (wpas_p2p_init(wpa_s->global, wpa_s) < 0) {
+			wpa_msg(wpa_s, MSG_ERROR, "Failed to init P2P");
+			return -1;
+		}
 	}
 
 	if (wpa_bss_init(wpa_s) < 0)