Patchwork Omit P2P Group Info, in case of no connected peers

login
register
mail settings
Submitter Chaitanya TK
Date March 19, 2013, 7:26 a.m.
Message ID <51481330.8010408@gmail.com>
Download mbox | patch
Permalink /patch/228926/
State Superseded
Headers show

Comments

Chaitanya TK - March 19, 2013, 7:26 a.m.
As per P2P-sec V1.2: "The P2P Group
Info attribute shall be omitted if there are zero
connected P2P Clients."

Don't add the IE, if the no of peers are zero.

Signed-off-by: T Krushna Chaitanya <chaitanyatk@posedge.com>
---

 src/p2p/p2p_group.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)
Jan Ceuleers - March 19, 2013, 8:35 p.m.
One review comment below.

On 03/19/2013 08:26 AM, Chaitanya TK wrote:
> 
> 
> As per P2P-sec V1.2: "The P2P Group
> Info attribute shall be omitted if there are zero
> connected P2P Clients."
> 
> Don't add the IE, if the no of peers are zero.
> 
> Signed-off-by: T Krushna Chaitanya <chaitanyatk@posedge.com>
> ---
> 
>  src/p2p/p2p_group.c |   16 +++++++++-------
>  1 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/src/p2p/p2p_group.c b/src/p2p/p2p_group.c
> index 9559e44..7adab90 100644
> --- a/src/p2p/p2p_group.c
> +++ b/src/p2p/p2p_group.c
> @@ -413,14 +413,16 @@ static struct wpabuf * p2p_group_build_probe_resp_ie(struct p2p_group *group)
>  	/* P2P Device Info */
>  	p2p_buf_add_device_info(p2p_subelems, group->p2p, NULL);
>  
> -	/* P2P Group Info */
> -	group_info = wpabuf_put(p2p_subelems, 0);
> -	wpabuf_put_u8(p2p_subelems, P2P_ATTR_GROUP_INFO);
> -	wpabuf_put_le16(p2p_subelems, 0); /* Length to be filled */
> -	for (m = group->members; m; m = m->next)
> -		p2p_client_info(p2p_subelems, m);
> -	WPA_PUT_LE16(group_info + 1,
> +	/* P2P Group Info: Only when at least 1 P2P CLI is connected */
> +	if (group->members != 0) {
> +		group_info = wpabuf_put(p2p_subelems, 0);
> +		wpabuf_put_u8(p2p_subelems, P2P_ATTR_GROUP_INFO);
> +		wpabuf_put_le16(p2p_subelems, 0); /* Length to be filled */
> +		for (m = group->members; m; m = m->next)
> +			p2p_client_info(p2p_subelems, m);
> +		WPA_PUT_LE16(group_info + 1,
>  		     (u8 *) wpabuf_put(p2p_subelems, 0) - group_info - 3);

Purely based on visual inspection I rather suspect that there should be
a "+" at the beginning of the above line.

How did you generate this patch?

> +	}
>  
>  	ie = p2p_group_encaps_probe_resp(p2p_subelems);
>  	wpabuf_free(p2p_subelems);

Thanks, Jan
Arend van Spriel - March 19, 2013, 8:51 p.m.
On 03/19/2013 09:35 PM, Jan Ceuleers wrote:
> One review comment below.
>
> On 03/19/2013 08:26 AM, Chaitanya TK wrote:
>>
>>
>> As per P2P-sec V1.2: "The P2P Group
>> Info attribute shall be omitted if there are zero
>> connected P2P Clients."
>>
>> Don't add the IE, if the no of peers are zero.
>>
>> Signed-off-by: T Krushna Chaitanya <chaitanyatk@posedge.com>

Should be: Signed-hostap:

See CONTRIBUTIONS file.

Also the email address does not match the gmail address you used to send 
it. Not sure if that is acceptable.

>> ---
>>
>>   src/p2p/p2p_group.c |   16 +++++++++-------
>>   1 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/p2p/p2p_group.c b/src/p2p/p2p_group.c
>> index 9559e44..7adab90 100644
>> --- a/src/p2p/p2p_group.c
>> +++ b/src/p2p/p2p_group.c
>> @@ -413,14 +413,16 @@ static struct wpabuf * p2p_group_build_probe_resp_ie(struct p2p_group *group)
>>   	/* P2P Device Info */
>>   	p2p_buf_add_device_info(p2p_subelems, group->p2p, NULL);
>>
>> -	/* P2P Group Info */
>> -	group_info = wpabuf_put(p2p_subelems, 0);
>> -	wpabuf_put_u8(p2p_subelems, P2P_ATTR_GROUP_INFO);
>> -	wpabuf_put_le16(p2p_subelems, 0); /* Length to be filled */
>> -	for (m = group->members; m; m = m->next)
>> -		p2p_client_info(p2p_subelems, m);
>> -	WPA_PUT_LE16(group_info + 1,
>> +	/* P2P Group Info: Only when at least 1 P2P CLI is connected */
>> +	if (group->members != 0) {
>> +		group_info = wpabuf_put(p2p_subelems, 0);
>> +		wpabuf_put_u8(p2p_subelems, P2P_ATTR_GROUP_INFO);
>> +		wpabuf_put_le16(p2p_subelems, 0); /* Length to be filled */
>> +		for (m = group->members; m; m = m->next)
>> +			p2p_client_info(p2p_subelems, m);
>> +		WPA_PUT_LE16(group_info + 1,
>>   		     (u8 *) wpabuf_put(p2p_subelems, 0) - group_info - 3);
>
> Purely based on visual inspection I rather suspect that there should be
> a "+" at the beginning of the above line.
>
> How did you generate this patch?
>

Might be an email client issue.

Regards,
Arend
Chaitanya TK - March 20, 2013, 7:08 a.m.
On Wed, Mar 20, 2013 at 2:21 AM, Arend van Spriel <arend@broadcom.com>wrote:

> On 03/19/2013 09:35 PM, Jan Ceuleers wrote:
>
>> One review comment below.
>>
>> On 03/19/2013 08:26 AM, Chaitanya TK wrote:
>>
>>>
>>>
>>> As per P2P-sec V1.2: "The P2P Group
>>> Info attribute shall be omitted if there are zero
>>> connected P2P Clients."
>>>
>>> Don't add the IE, if the no of peers are zero.
>>>
>>> Signed-off-by: T Krushna Chaitanya <chaitanyatk@posedge.com>
>>>
>>
> Should be: Signed-hostap:
>
> See CONTRIBUTIONS file.
>

Will resend the patch with signed-off changed.


>
> Also the email address does not match the gmail address you used to send
> it. Not sure if that is acceptable.


The problem with the email is that i want to store all
the group emails to my gmail for future use but as we
cant use gmail to send patches (line wrapping and whitespace
issues) i used my official email(uses thuderbird)
which is not registered, so i use SMTP of gmail through
thunderbird causing the confusion.

>
>
>  ---
>>>
>>>   src/p2p/p2p_group.c |   16 +++++++++-------
>>>   1 files changed, 9 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/p2p/p2p_group.c b/src/p2p/p2p_group.c
>>> index 9559e44..7adab90 100644
>>> --- a/src/p2p/p2p_group.c
>>> +++ b/src/p2p/p2p_group.c
>>> @@ -413,14 +413,16 @@ static struct wpabuf *
>>> p2p_group_build_probe_resp_ie(**struct p2p_group *group)
>>>         /* P2P Device Info */
>>>         p2p_buf_add_device_info(p2p_**subelems, group->p2p, NULL);
>>>
>>> -       /* P2P Group Info */
>>> -       group_info = wpabuf_put(p2p_subelems, 0);
>>> -       wpabuf_put_u8(p2p_subelems, P2P_ATTR_GROUP_INFO);
>>> -       wpabuf_put_le16(p2p_subelems, 0); /* Length to be filled */
>>> -       for (m = group->members; m; m = m->next)
>>> -               p2p_client_info(p2p_subelems, m);
>>> -       WPA_PUT_LE16(group_info + 1,
>>> +       /* P2P Group Info: Only when at least 1 P2P CLI is connected */
>>> +       if (group->members != 0) {
>>> +               group_info = wpabuf_put(p2p_subelems, 0);
>>> +               wpabuf_put_u8(p2p_subelems, P2P_ATTR_GROUP_INFO);
>>> +               wpabuf_put_le16(p2p_subelems, 0); /* Length to be filled
>>> */
>>> +               for (m = group->members; m; m = m->next)
>>> +                       p2p_client_info(p2p_subelems, m);
>>> +               WPA_PUT_LE16(group_info + 1,
>>>                      (u8 *) wpabuf_put(p2p_subelems, 0) - group_info -
>>> 3);
>>>
>>
>> Purely based on visual inspection I rather suspect that there should be
>> a "+" at the beginning of the above line.
>>
>> How did you generate this patch?
>>
>>
> Might be an email client issue.
>

I have used git diff to generate the patch, let
me check the issue with that line.
Arend van Spriel - March 20, 2013, 11:30 a.m.
On 03/20/2013 08:08 AM, Krishna Chaitanya wrote:
> > >   How did you generate this patch?
> > >
> >
> >     Might be an email client issue.
>
> I have used git diff to generate the patch, let
> me check the issue with that line.

You can use 'git format-patch' which will create a .patch file. This can 
be sent using 'git send-email' provided you have a working sendmail utility.

Gr. AvS
Chaitanya TK - March 20, 2013, 12:20 p.m.
On Wed, Mar 20, 2013 at 5:00 PM, Arend van Spriel <arend@broadcom.com>wrote:

> On 03/20/2013 08:08 AM, Krishna Chaitanya wrote:
>
>> > >   How did you generate this patch?
>> > >
>> >
>> >     Might be an email client issue.
>>
>> I have used git diff to generate the patch, let
>> me check the issue with that line.
>>
>
> You can use 'git format-patch' which will create a .patch file. This can
> be sent using 'git send-email' provided you have a working sendmail utility.
>

Sure, will  try that.

Patch

diff --git a/src/p2p/p2p_group.c b/src/p2p/p2p_group.c
index 9559e44..7adab90 100644
--- a/src/p2p/p2p_group.c
+++ b/src/p2p/p2p_group.c
@@ -413,14 +413,16 @@  static struct wpabuf * p2p_group_build_probe_resp_ie(struct p2p_group *group)
 	/* P2P Device Info */
 	p2p_buf_add_device_info(p2p_subelems, group->p2p, NULL);
 
-	/* P2P Group Info */
-	group_info = wpabuf_put(p2p_subelems, 0);
-	wpabuf_put_u8(p2p_subelems, P2P_ATTR_GROUP_INFO);
-	wpabuf_put_le16(p2p_subelems, 0); /* Length to be filled */
-	for (m = group->members; m; m = m->next)
-		p2p_client_info(p2p_subelems, m);
-	WPA_PUT_LE16(group_info + 1,
+	/* P2P Group Info: Only when at least 1 P2P CLI is connected */
+	if (group->members != 0) {
+		group_info = wpabuf_put(p2p_subelems, 0);
+		wpabuf_put_u8(p2p_subelems, P2P_ATTR_GROUP_INFO);
+		wpabuf_put_le16(p2p_subelems, 0); /* Length to be filled */
+		for (m = group->members; m; m = m->next)
+			p2p_client_info(p2p_subelems, m);
+		WPA_PUT_LE16(group_info + 1,
 		     (u8 *) wpabuf_put(p2p_subelems, 0) - group_info - 3);
+	}
 
 	ie = p2p_group_encaps_probe_resp(p2p_subelems);
 	wpabuf_free(p2p_subelems);