RE: [PATCH] monitor: Fix AVRCP GetElementAttributes formatting issue

[Vikrampal <vikram.pal@samsung.com>]

Fine.

> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> Sent: Tuesday, November 11, 2014 4:39 PM
> To: Vikrampal
> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; cpgs@samsung.com
> Subject: Re: [PATCH] monitor: Fix AVRCP GetElementAttributes formatting
> issue
> 
> Hi Vikram,
> 
> On Tue, Nov 11, 2014 at 12:15 PM, Vikrampal <vikram.pal@samsung.com>
> wrote:
> > Hi Luiz,
> >
> >> -----Original Message-----
> >> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> >> Sent: Tuesday, November 11, 2014 3:07 PM
> >> To: Vikrampal Yadav
> >> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin;
> >> cpgs@samsung.com
> >> Subject: Re: [PATCH] monitor: Fix AVRCP GetElementAttributes
> >> formatting issue
> >>
> >> Hi Vikram,
> >>
> >> On Wed, Nov 5, 2014 at 3:42 PM, Vikrampal Yadav
> >> <vikram.pal@samsung.com> 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