2014-11-05 13:42:39

by Vikrampal Yadav

[permalink] [raw]
Subject: [PATCH] monitor: Fix AVRCP GetElementAttributes formatting issue

AVRCP GetElementAttributes formatting issue for AttributeValue fixed.

AVCTP Control: Response: type 0x00 label 4 PID 0x110e
AV/C: Stable: address 0x48 opcode 0x00
Subunit: Panel
Opcode: Vendor Dependent
Company ID: 0x001958
AVRCP: GetElementAttributes pt Single len 0x0040
AttributeCount: 0x04
Attribute: 0x00000001 (Title)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x0008
AttributeValue: fernando
Attribute: 0x00000002 (Artist)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x0004
AttributeValue: abba
Attribute: 0x00000003 (Album)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x000d
AttributeValue: Greatest Hits
Attribute: 0x00000007 (Track duration)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x0006
AttributeValue: 150000
---
monitor/avctp.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/monitor/avctp.c b/monitor/avctp.c
index 4abd18f..11579ad 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -1122,7 +1122,9 @@ response:
num = avrcp_continuing.num;

if (avrcp_continuing.size > 0) {
+ char attrval[1024];
uint16_t size;
+ uint8_t idx;

if (avrcp_continuing.size > len) {
size = len;
@@ -1132,16 +1134,17 @@ response:
avrcp_continuing.size = 0;
}

- printf("ContinuingAttributeValue: ");
- for (; size > 0; size--) {
+ for (idx = 0; size > 0; idx++, size--) {
uint8_t c;

if (!l2cap_frame_get_u8(frame, &c))
goto failed;

- printf("%1c", isprint(c) ? c : '.');
+ sprintf(&attrval[idx], "%1c",
+ isprint(c) ? c : '.');
}
- printf("\n");
+ print_field("%*cContinuingAttributeValue: %s",
+ (indent - 8), ' ', attrval);

len -= size;
}
@@ -1153,6 +1156,8 @@ response:
while (num > 0 && len > 0) {
uint32_t attr;
uint16_t charset, attrlen;
+ uint8_t idx;
+ char attrval[1024];

if (!l2cap_frame_get_be32(frame, &attr))
goto failed;
@@ -1175,15 +1180,16 @@ response:
len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
num--;

- print_field("%*cAttributeValue: ", (indent - 8), ' ');
- for (; attrlen > 0 && len > 0; attrlen--, len--) {
+ for (idx = 0; attrlen > 0 && len > 0; idx++, attrlen--, len--) {
uint8_t c;

if (!l2cap_frame_get_u8(frame, &c))
goto failed;

- printf("%1c", isprint(c) ? c : '.');
+ sprintf(&attrval[idx], "%1c", isprint(c) ? c : '.');
}
+ print_field("%*cAttributeValue: %s", (indent - 8),
+ ' ', attrval);

if (attrlen > 0)
avrcp_continuing.size = attrlen;
--
1.9.1



2014-11-11 11:13:53

by Vikrampal Yadav

[permalink] [raw]
Subject: RE: [PATCH] monitor: Fix AVRCP GetElementAttributes formatting issue

Fine.

> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:[email protected]]
> Sent: Tuesday, November 11, 2014 4:39 PM
> To: Vikrampal
> Cc: [email protected]; Dmitry Kasatkin; [email protected]
> Subject: Re: [PATCH] monitor: Fix AVRCP GetElementAttributes formatting
> issue
>
> Hi Vikram,
>
> On Tue, Nov 11, 2014 at 12:15 PM, Vikrampal <[email protected]>
> wrote:
> > Hi Luiz,
> >
> >> -----Original Message-----
> >> From: Luiz Augusto von Dentz [mailto:[email protected]]
> >> Sent: Tuesday, November 11, 2014 3:07 PM
> >> To: Vikrampal Yadav
> >> Cc: [email protected]; Dmitry Kasatkin;
> >> [email protected]
> >> Subject: Re: [PATCH] monitor: Fix AVRCP GetElementAttributes
> >> formatting issue
> >>
> >> Hi Vikram,
> >>
> >> On Wed, Nov 5, 2014 at 3:42 PM, Vikrampal Yadav
> >> <[email protected]> wrote:
> >> > AVRCP GetElementAttributes formatting issue for AttributeValue fixed.
> >> >
> >> > AVCTP Control: Response: type 0x00 label 4 PID 0x110e
> >> > AV/C: Stable: address 0x48 opcode 0x00
> >> > Subunit: Panel
> >> > Opcode: Vendor Dependent
> >> > Company ID: 0x001958
> >> > AVRCP: GetElementAttributes pt Single len 0x0040
> >> > AttributeCount: 0x04
> >> > Attribute: 0x00000001 (Title)
> >> > CharsetID: 0x006a (UTF-8)
> >> > AttributeValueLength: 0x0008
> >> > AttributeValue: fernando
> >> > Attribute: 0x00000002 (Artist)
> >> > CharsetID: 0x006a (UTF-8)
> >> > AttributeValueLength: 0x0004
> >> > AttributeValue: abba
> >> > Attribute: 0x00000003 (Album)
> >> > CharsetID: 0x006a (UTF-8)
> >> > AttributeValueLength: 0x000d
> >> > AttributeValue: Greatest Hits
> >> > Attribute: 0x00000007 (Track duration)
> >> > CharsetID: 0x006a (UTF-8)
> >> > AttributeValueLength: 0x0006
> >> > AttributeValue: 150000
> >>
> >> I don't get it, you seem to be changing ContinuingAttributeValue yet
> >> the frame above does not contain it?
> >
> > This fix is also for AttributeValue. While reviewing found issue with
> > ContinuingAttributeValue as well so fixed both.
>
> Ok, then please fix the buffer size.
>
> >> > ---
> >> > monitor/avctp.c | 20 +++++++++++++-------
> >> > 1 file changed, 13 insertions(+), 7 deletions(-)
> >> >
> >> > diff --git a/monitor/avctp.c b/monitor/avctp.c index
> >> > 4abd18f..11579ad
> >> > 100644
> >> > --- a/monitor/avctp.c
> >> > +++ b/monitor/avctp.c
> >> > @@ -1122,7 +1122,9 @@ response:
> >> > num = avrcp_continuing.num;
> >> >
> >> > if (avrcp_continuing.size > 0) {
> >> > + char attrval[1024];
> >>
> >> The buffer should be as big as size can be which is UINT8_MAX.
> >>
> >> > uint16_t size;
> >> > + uint8_t idx;
> >> >
> >> > if (avrcp_continuing.size > len) {
> >> > size = len; @@ -1132,16 +1134,17 @@
> >> > response:
> >> > avrcp_continuing.size = 0;
> >> > }
> >> >
> >> > - printf("ContinuingAttributeValue: ");
> >> > - for (; size > 0; size--) {
> >> > + for (idx = 0; size > 0; idx++, size--) {
> >> > uint8_t c;
> >> >
> >> > if (!l2cap_frame_get_u8(frame, &c))
> >> > goto failed;
> >> >
> >> > - printf("%1c", isprint(c) ? c : '.');
> >> > + sprintf(&attrval[idx], "%1c",
> >> > + isprint(c)
> >> > + ? c
> >> > + : '.');
> >> > }
> >> > - printf("\n");
> >> > + print_field("%*cContinuingAttributeValue: %s",
> >> > + (indent - 8), ' ',
> >> > + attrval);
> >> >
> >> > len -= size;
> >> > }
> >> > @@ -1153,6 +1156,8 @@ response:
> >> > while (num > 0 && len > 0) {
> >> > uint32_t attr;
> >> > uint16_t charset, attrlen;
> >> > + uint8_t idx;
> >> > + char attrval[1024];
> >>
> >> Same here, use UINT8_MAX.
> >>
> >> > if (!l2cap_frame_get_be32(frame, &attr))
> >> > goto failed; @@ -1175,15 +1180,16 @@
> >> > response:
> >> > len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
> >> > num--;
> >> >
> >> > - print_field("%*cAttributeValue: ", (indent - 8), ' ');
> >> > - for (; attrlen > 0 && len > 0; attrlen--, len--) {
> >> > + for (idx = 0; attrlen > 0 && len > 0; idx++,
> >> > + attrlen--, len--) {
> >> > uint8_t c;
> >> >
> >> > if (!l2cap_frame_get_u8(frame, &c))
> >> > goto failed;
> >> >
> >> > - printf("%1c", isprint(c) ? c : '.');
> >> > + sprintf(&attrval[idx], "%1c", isprint(c) ? c :
> >> > + '.');
> >> > }
> >> > + print_field("%*cAttributeValue: %s", (indent - 8),
> >> > + '
> >> > + ', attrval);
> >> >
> >> > if (attrlen > 0)
> >> > avrcp_continuing.size = attrlen;
> >> > --
> >> > 1.9.1
> >> >
> >>
> >>
> >>
> >> --
> >> Luiz Augusto von Dentz
> >
> > Regards,
> > Vikram
> >
>
>
>
> --
> Luiz Augusto von Dentz

Vikram


2014-11-11 11:08:49

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] monitor: Fix AVRCP GetElementAttributes formatting issue

Hi Vikram,

On Tue, Nov 11, 2014 at 12:15 PM, Vikrampal <[email protected]> wrote:
> Hi Luiz,
>
>> -----Original Message-----
>> From: Luiz Augusto von Dentz [mailto:[email protected]]
>> Sent: Tuesday, November 11, 2014 3:07 PM
>> To: Vikrampal Yadav
>> Cc: [email protected]; Dmitry Kasatkin; [email protected]
>> Subject: Re: [PATCH] monitor: Fix AVRCP GetElementAttributes formatting
>> issue
>>
>> Hi Vikram,
>>
>> On Wed, Nov 5, 2014 at 3:42 PM, Vikrampal Yadav
>> <[email protected]> wrote:
>> > AVRCP GetElementAttributes formatting issue for AttributeValue fixed.
>> >
>> > AVCTP Control: Response: type 0x00 label 4 PID 0x110e
>> > AV/C: Stable: address 0x48 opcode 0x00
>> > Subunit: Panel
>> > Opcode: Vendor Dependent
>> > Company ID: 0x001958
>> > AVRCP: GetElementAttributes pt Single len 0x0040
>> > AttributeCount: 0x04
>> > Attribute: 0x00000001 (Title)
>> > CharsetID: 0x006a (UTF-8)
>> > AttributeValueLength: 0x0008
>> > AttributeValue: fernando
>> > Attribute: 0x00000002 (Artist)
>> > CharsetID: 0x006a (UTF-8)
>> > AttributeValueLength: 0x0004
>> > AttributeValue: abba
>> > Attribute: 0x00000003 (Album)
>> > CharsetID: 0x006a (UTF-8)
>> > AttributeValueLength: 0x000d
>> > AttributeValue: Greatest Hits
>> > Attribute: 0x00000007 (Track duration)
>> > CharsetID: 0x006a (UTF-8)
>> > AttributeValueLength: 0x0006
>> > AttributeValue: 150000
>>
>> I don't get it, you seem to be changing ContinuingAttributeValue yet the
>> frame above does not contain it?
>
> This fix is also for AttributeValue. While reviewing found issue with ContinuingAttributeValue
> as well so fixed both.

Ok, then please fix the buffer size.

>> > ---
>> > monitor/avctp.c | 20 +++++++++++++-------
>> > 1 file changed, 13 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/monitor/avctp.c b/monitor/avctp.c index 4abd18f..11579ad
>> > 100644
>> > --- a/monitor/avctp.c
>> > +++ b/monitor/avctp.c
>> > @@ -1122,7 +1122,9 @@ response:
>> > num = avrcp_continuing.num;
>> >
>> > if (avrcp_continuing.size > 0) {
>> > + char attrval[1024];
>>
>> The buffer should be as big as size can be which is UINT8_MAX.
>>
>> > uint16_t size;
>> > + uint8_t idx;
>> >
>> > if (avrcp_continuing.size > len) {
>> > size = len; @@ -1132,16 +1134,17 @@
>> > response:
>> > avrcp_continuing.size = 0;
>> > }
>> >
>> > - printf("ContinuingAttributeValue: ");
>> > - for (; size > 0; size--) {
>> > + for (idx = 0; size > 0; idx++, size--) {
>> > uint8_t c;
>> >
>> > if (!l2cap_frame_get_u8(frame, &c))
>> > goto failed;
>> >
>> > - printf("%1c", isprint(c) ? c : '.');
>> > + sprintf(&attrval[idx], "%1c",
>> > + isprint(c) ? c
>> > + : '.');
>> > }
>> > - printf("\n");
>> > + print_field("%*cContinuingAttributeValue: %s",
>> > + (indent - 8), ' ',
>> > + attrval);
>> >
>> > len -= size;
>> > }
>> > @@ -1153,6 +1156,8 @@ response:
>> > while (num > 0 && len > 0) {
>> > uint32_t attr;
>> > uint16_t charset, attrlen;
>> > + uint8_t idx;
>> > + char attrval[1024];
>>
>> Same here, use UINT8_MAX.
>>
>> > if (!l2cap_frame_get_be32(frame, &attr))
>> > goto failed;
>> > @@ -1175,15 +1180,16 @@ response:
>> > len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
>> > num--;
>> >
>> > - print_field("%*cAttributeValue: ", (indent - 8), ' ');
>> > - for (; attrlen > 0 && len > 0; attrlen--, len--) {
>> > + for (idx = 0; attrlen > 0 && len > 0; idx++,
>> > + attrlen--, len--) {
>> > uint8_t c;
>> >
>> > if (!l2cap_frame_get_u8(frame, &c))
>> > goto failed;
>> >
>> > - printf("%1c", isprint(c) ? c : '.');
>> > + sprintf(&attrval[idx], "%1c", isprint(c) ? c :
>> > + '.');
>> > }
>> > + print_field("%*cAttributeValue: %s", (indent - 8),
>> > + ' ',
>> > + attrval);
>> >
>> > if (attrlen > 0)
>> > avrcp_continuing.size = attrlen;
>> > --
>> > 1.9.1
>> >
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
> Regards,
> Vikram
>



--
Luiz Augusto von Dentz

2014-11-11 10:15:12

by Vikrampal Yadav

[permalink] [raw]
Subject: RE: [PATCH] monitor: Fix AVRCP GetElementAttributes formatting issue

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:[email protected]]
> Sent: Tuesday, November 11, 2014 3:07 PM
> To: Vikrampal Yadav
> Cc: [email protected]; Dmitry Kasatkin; [email protected]
> Subject: Re: [PATCH] monitor: Fix AVRCP GetElementAttributes formatting
> issue
>
> Hi Vikram,
>
> On Wed, Nov 5, 2014 at 3:42 PM, Vikrampal Yadav
> <[email protected]> wrote:
> > AVRCP GetElementAttributes formatting issue for AttributeValue fixed.
> >
> > AVCTP Control: Response: type 0x00 label 4 PID 0x110e
> > AV/C: Stable: address 0x48 opcode 0x00
> > Subunit: Panel
> > Opcode: Vendor Dependent
> > Company ID: 0x001958
> > AVRCP: GetElementAttributes pt Single len 0x0040
> > AttributeCount: 0x04
> > Attribute: 0x00000001 (Title)
> > CharsetID: 0x006a (UTF-8)
> > AttributeValueLength: 0x0008
> > AttributeValue: fernando
> > Attribute: 0x00000002 (Artist)
> > CharsetID: 0x006a (UTF-8)
> > AttributeValueLength: 0x0004
> > AttributeValue: abba
> > Attribute: 0x00000003 (Album)
> > CharsetID: 0x006a (UTF-8)
> > AttributeValueLength: 0x000d
> > AttributeValue: Greatest Hits
> > Attribute: 0x00000007 (Track duration)
> > CharsetID: 0x006a (UTF-8)
> > AttributeValueLength: 0x0006
> > AttributeValue: 150000
>
> I don't get it, you seem to be changing ContinuingAttributeValue yet the
> frame above does not contain it?

This fix is also for AttributeValue. While reviewing found issue with ContinuingAttributeValue
as well so fixed both.
> > ---
> > monitor/avctp.c | 20 +++++++++++++-------
> > 1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/monitor/avctp.c b/monitor/avctp.c index 4abd18f..11579ad
> > 100644
> > --- a/monitor/avctp.c
> > +++ b/monitor/avctp.c
> > @@ -1122,7 +1122,9 @@ response:
> > num = avrcp_continuing.num;
> >
> > if (avrcp_continuing.size > 0) {
> > + char attrval[1024];
>
> The buffer should be as big as size can be which is UINT8_MAX.
>
> > uint16_t size;
> > + uint8_t idx;
> >
> > if (avrcp_continuing.size > len) {
> > size = len; @@ -1132,16 +1134,17 @@
> > response:
> > avrcp_continuing.size = 0;
> > }
> >
> > - printf("ContinuingAttributeValue: ");
> > - for (; size > 0; size--) {
> > + for (idx = 0; size > 0; idx++, size--) {
> > uint8_t c;
> >
> > if (!l2cap_frame_get_u8(frame, &c))
> > goto failed;
> >
> > - printf("%1c", isprint(c) ? c : '.');
> > + sprintf(&attrval[idx], "%1c",
> > + isprint(c) ? c
> > + : '.');
> > }
> > - printf("\n");
> > + print_field("%*cContinuingAttributeValue: %s",
> > + (indent - 8), ' ',
> > + attrval);
> >
> > len -= size;
> > }
> > @@ -1153,6 +1156,8 @@ response:
> > while (num > 0 && len > 0) {
> > uint32_t attr;
> > uint16_t charset, attrlen;
> > + uint8_t idx;
> > + char attrval[1024];
>
> Same here, use UINT8_MAX.
>
> > if (!l2cap_frame_get_be32(frame, &attr))
> > goto failed;
> > @@ -1175,15 +1180,16 @@ response:
> > len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
> > num--;
> >
> > - print_field("%*cAttributeValue: ", (indent - 8), ' ');
> > - for (; attrlen > 0 && len > 0; attrlen--, len--) {
> > + for (idx = 0; attrlen > 0 && len > 0; idx++,
> > + attrlen--, len--) {
> > uint8_t c;
> >
> > if (!l2cap_frame_get_u8(frame, &c))
> > goto failed;
> >
> > - printf("%1c", isprint(c) ? c : '.');
> > + sprintf(&attrval[idx], "%1c", isprint(c) ? c :
> > + '.');
> > }
> > + print_field("%*cAttributeValue: %s", (indent - 8),
> > + ' ',
> > + attrval);
> >
> > if (attrlen > 0)
> > avrcp_continuing.size = attrlen;
> > --
> > 1.9.1
> >
>
>
>
> --
> Luiz Augusto von Dentz

Regards,
Vikram


2014-11-11 09:37:10

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] monitor: Fix AVRCP GetElementAttributes formatting issue

Hi Vikram,

On Wed, Nov 5, 2014 at 3:42 PM, Vikrampal Yadav <[email protected]> wrote:
> AVRCP GetElementAttributes formatting issue for AttributeValue fixed.
>
> AVCTP Control: Response: type 0x00 label 4 PID 0x110e
> AV/C: Stable: address 0x48 opcode 0x00
> Subunit: Panel
> Opcode: Vendor Dependent
> Company ID: 0x001958
> AVRCP: GetElementAttributes pt Single len 0x0040
> AttributeCount: 0x04
> Attribute: 0x00000001 (Title)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x0008
> AttributeValue: fernando
> Attribute: 0x00000002 (Artist)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x0004
> AttributeValue: abba
> Attribute: 0x00000003 (Album)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x000d
> AttributeValue: Greatest Hits
> Attribute: 0x00000007 (Track duration)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x0006
> AttributeValue: 150000

I don't get it, you seem to be changing ContinuingAttributeValue yet
the frame above does not contain it?

> ---
> monitor/avctp.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/monitor/avctp.c b/monitor/avctp.c
> index 4abd18f..11579ad 100644
> --- a/monitor/avctp.c
> +++ b/monitor/avctp.c
> @@ -1122,7 +1122,9 @@ response:
> num = avrcp_continuing.num;
>
> if (avrcp_continuing.size > 0) {
> + char attrval[1024];

The buffer should be as big as size can be which is UINT8_MAX.

> uint16_t size;
> + uint8_t idx;
>
> if (avrcp_continuing.size > len) {
> size = len;
> @@ -1132,16 +1134,17 @@ response:
> avrcp_continuing.size = 0;
> }
>
> - printf("ContinuingAttributeValue: ");
> - for (; size > 0; size--) {
> + for (idx = 0; size > 0; idx++, size--) {
> uint8_t c;
>
> if (!l2cap_frame_get_u8(frame, &c))
> goto failed;
>
> - printf("%1c", isprint(c) ? c : '.');
> + sprintf(&attrval[idx], "%1c",
> + isprint(c) ? c : '.');
> }
> - printf("\n");
> + print_field("%*cContinuingAttributeValue: %s",
> + (indent - 8), ' ', attrval);
>
> len -= size;
> }
> @@ -1153,6 +1156,8 @@ response:
> while (num > 0 && len > 0) {
> uint32_t attr;
> uint16_t charset, attrlen;
> + uint8_t idx;
> + char attrval[1024];

Same here, use UINT8_MAX.

> if (!l2cap_frame_get_be32(frame, &attr))
> goto failed;
> @@ -1175,15 +1180,16 @@ response:
> len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
> num--;
>
> - print_field("%*cAttributeValue: ", (indent - 8), ' ');
> - for (; attrlen > 0 && len > 0; attrlen--, len--) {
> + for (idx = 0; attrlen > 0 && len > 0; idx++, attrlen--, len--) {
> uint8_t c;
>
> if (!l2cap_frame_get_u8(frame, &c))
> goto failed;
>
> - printf("%1c", isprint(c) ? c : '.');
> + sprintf(&attrval[idx], "%1c", isprint(c) ? c : '.');
> }
> + print_field("%*cAttributeValue: %s", (indent - 8),
> + ' ', attrval);
>
> if (attrlen > 0)
> avrcp_continuing.size = attrlen;
> --
> 1.9.1
>



--
Luiz Augusto von Dentz