Re: [PATCH BlueZ 03/16] android/avrcp-lib: Rework handler callback parameters

[Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>]

Hi Andrei,

On Mon, Mar 3, 2014 at 11:52 AM, Andrei Emeltchenko
<andrei.emeltchenko.news@xxxxxxxxx> wrote:
> Hi Luiz,
>
> On Sun, Mar 02, 2014 at 08:48:18PM +0200, Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>>
>> This rework handler callback parameters to make it able to return
>> proper error such as -EAGAIN to implement asynchronous responses.
>> ---
>>  android/avrcp-lib.c | 12 +++++++++---
>>  android/avrcp-lib.h |  5 +++--
>>  unit/test-avrcp.c   | 42 +++++++++++++++++++++---------------------
>>  3 files changed, 33 insertions(+), 26 deletions(-)
>>
>> diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
>> index cdad6ac..339be16 100644
>> --- a/android/avrcp-lib.c
>> +++ b/android/avrcp-lib.c
>> @@ -125,6 +125,7 @@ static ssize_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
>>       struct avrcp_header *pdu = (void *) operands;
>>       uint32_t company_id = ntoh24(pdu->company_id);
>>       uint16_t params_len = ntohs(pdu->params_len);
>> +     ssize_t ret;
>>
>>       if (company_id != IEEEID_BTSIG) {
>>               *code = AVC_CTYPE_NOT_IMPLEMENTED;
>> @@ -159,12 +160,17 @@ static ssize_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
>>               goto reject;
>>       }
>>
>> -     *code = handler->func(session, transaction, &params_len,
>> +     ret = handler->func(session, transaction, code, params_len,
>>                                       pdu->params, session->control_data);
>> +     if (ret < 0) {
>> +             if (ret == -EAGAIN)
>> +                     return ret;
>> +             goto reject;
>> +     }
>>
>> -     pdu->params_len = htons(params_len);
>> +     pdu->params_len = htons(ret);
>>
>> -     return AVRCP_HEADER_LENGTH + params_len;
>> +     return AVRCP_HEADER_LENGTH + ret;
>>
>>  reject:
>>       pdu->params_len = htons(1);
>> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
>> index a33bdfe..d7a805b 100644
>> --- a/android/avrcp-lib.h
>> +++ b/android/avrcp-lib.h
>> @@ -79,8 +79,9 @@ struct avrcp;
>>  struct avrcp_control_handler {
>>       uint8_t id;
>>       uint8_t code;
>> -     uint8_t (*func) (struct avrcp *session, uint8_t transaction,
>> -                     uint16_t *params_len, uint8_t *params, void *user_data);
>> +     ssize_t (*func) (struct avrcp *session, uint8_t transaction,
>> +                     uint8_t *code, uint16_t params_len, uint8_t *params,
>> +                     void *user_data);
>>  };
>>
>>  struct avrcp_passthrough_handler {
>> diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
>> index 302c331..2d5711b 100644
>> --- a/unit/test-avrcp.c
>> +++ b/unit/test-avrcp.c
>> @@ -292,54 +292,54 @@ static const struct avrcp_passthrough_handler passthrough_handlers[] = {
>>               { },
>>  };
>>
>> -static uint8_t avrcp_handle_get_capabilities(struct avrcp *session,
>> -                             uint8_t transaction, uint16_t *params_len,
>> -                             uint8_t *params, void *user_data)
>> +static ssize_t avrcp_handle_get_capabilities(struct avrcp *session,
>> +                                     uint8_t transaction, uint8_t *code,
>> +                                     uint16_t params_len, uint8_t *params,
>> +                                     void *user_data)
>>  {
>>       uint32_t id = 0x001958;
>>
>> -     DBG("params[0] %d params_len %d", params[0], *params_len);
>> -
>> -     if (*params_len != 1)
>> +     if (params_len != 1)
>>               goto fail;
>>
>>       switch (params[0]) {
>>       case CAP_COMPANY_ID:
>> -             *params_len = 5;
>>               params[1] = 1;
>>               hton24(&params[2], id);
>> -             return AVC_CTYPE_STABLE;
>> +             *code = AVC_CTYPE_STABLE;
>> +             return 5;
>
> What is 5? Shall we define constants or use formulas?

This is the param length, ideally we should define structs for all the PDUs.


-- 
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