Hi Michal,
On Tue, Jun 19, 2012 at 9:39 AM, <Michal.Labedzki@xxxxxxxxx> wrote:
> Hi Luiz,
>
> Some cosmetic comments.
>
>> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>>
>> ---
>> parser/avrcp.c | 35 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/parser/avrcp.c b/parser/avrcp.c
>> index 850741b..0f2f1e6 100644
>> --- a/parser/avrcp.c
>> +++ b/parser/avrcp.c
>> @@ -1222,6 +1222,38 @@ static void avrcp_set_absolute_volume_dump(int level, struct frame *frm,
>> printf("Volume: %.2f%% (%d/127)\n", value/1.27, value);
>> }
>>
>> +static void avrcp_set_addressed_player(int level, struct frame *frm,
>> + uint8_t ctype, uint16_t len)
>> +{
>> + uint16_t id;
>> + uint8_t status;
>> +
>> + p_indent(level, frm);
>> +
>> + if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
>> + goto response;
>
> This command type is only AVC_CTYPE_CONTROL with AVC_CTYPE_NOT_IMPLEMENTED, AVC_CTYPE_ACCEPTED, AVC_CTYPE_REJECTED responses, so maybe all other should be tread as malformed.
This is a dumping tool, so the purpose to print as much as possible.
>
>> +
>> + if (len < 2) {
>
> What do you think about replace "<" by "!="? Large packet probably is not correct. But this can be what it is.
Again the purpose is to print as much as possible.
>
>> + printf("PDU Malformed\n");
>> + raw_dump(level, frm);
>> + return;
>> + }
>> +
>> + id = get_u16(frm);
>> + printf("PlayerID: 0x%04x", id);
>> + return;
>
> ID in hex? Decimal seems to be more natural.
This was taken from the spec, but either way is fine.
--
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
