| Message ID | 51481330.8010408@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
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
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
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.
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
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.
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);
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(-)