Hi Marcel,
>> + if (!op) {
>> + /* There is no pending request so the response is unexpected.
>> + * TODO: In such a case, the connection should be dropped
>> + * entirely. For now, simply ignore the PDU.
>> + */
>
> is this what the specification say?
Actually, I can't seem to find the section in the spec that says this,
so I might be remembering it wrong. What it does say is that if a
request times out (after 30 seconds) then the ATT bearer should be
terminated. Once I add way to notify the upper layers of a request
time out (perhaps as a ATT Error OP with a special BlueZ specific
error code) we will technically be handling this case. I already added
a TODO for implementing timeouts.
> if (att->destroyed)
> return true;
>
> wakeup_writer(att);
>
> I prefer it this way since it is easier to read when you go through the function. Especially with the att->destroyed cases for the reentrant support. We want to make it clear that we expect that object to be gone.
I got rid of att->destroyed for now.
>> +static bool handle_error_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
>> + ssize_t pdu_len)
>> +{
>> + struct bt_att_error_rsp_param p;
>> + bool result;
>> +
>> + if (pdu_len != 5)
>> + return false;
>> +
>> + memset(&p, 0, sizeof(p));
>> + p.request_opcode = pdu[1];
>> + p.handle = get_le16(pdu + 2);
>> + p.error_code = pdu[4];
>> +
>> + /* Note: avoid a tail call */
>
> I do not get this comment. Explain please.
>
>> + result = request_complete(att, p.request_opcode, opcode, &p, sizeof(p));
>> + return result;
>
> return request_complete(..
>
Ah yes, if you do a tail call here (i.e. return request_complete(... )
the "struct bt_att_error_rsp_param p" will go out of scope before
request_complete executes, due to tail call optimization, so the
pointer to to "p" that I'm passing down to request_complete will be
invalid. Hence the workaround of calling before returning.
Cheers,
Arman
--
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
