Re: [PATCH v2 1/5] shared/att: Drop the connection if a request is received while one is pending.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Arman,

On Wed, Oct 15, 2014 at 8:36 PM, Arman Uguray <armansito@xxxxxxxxxxxx> wrote:
> Hi Luiz,
>
>
>>> +       case ATT_OP_TYPE_REQ:
>>> +               /* If a request is currently pending, then the sequential
>>> +                * protocol was violated. Disconnect the bearer and notify the
>>> +                * upper-layer.
>>> +                */
>>
>> It seems this code is not following our coding style regarding multi
>> line comments, check doc/coding-style.txt(M2: Multiple line comment)
>> the first line should be empty.
>>
>
> I didn't realize that was the comment style. I've seen both styles in
> daemon code and I think I went with this one because of that. I'd
> rather have the code be consistent (since shared/att uses this style
> elsewhere) and maybe do a style correction pass in another patch?

Sure, I can probably fix this myself just made the comment to let you
know that we do in fact have a coding style for it.

>
>>> +               if (att->in_req) {
>>> +                       util_debug(att->debug_callback, att->debug_data,
>>> +                                       "Received request while another is "
>>> +                                       "pending: 0x%02x", opcode);
>>> +                       disconnect_cb(att->io, att);
>>> +                       return false;
>>> +               }
>>> +
>>> +               att->in_req = true;
>>> +
>>> +               /* Fall through to the next case */
>>
>> This might cause a problem with our own code when connecting to each
>> other if bt_att_cancel is used, apparently bt_att_cancel does not send
>> anything to the remote side which seems to enable sending a new
>> request without waiting any response for the first one.
>>
>
> I'm confused. Do you mean if bt_att is being used in the client role,
> it can send multiple requests this way? That is correct, though I
> don't see how that's related to this patch? Currently bt_att_cancel
> can be used to cancel waiting for a pending request and send the next
> one, as you said. In that regard, it doesn't actually "cancel"
> anything if the PDU has been sent over the air already. I don't know
> how big of a problem this is, since the remote end should normally
> detect the error and terminate the connection anyway. If this becomes
> a problem we can always prevent canceling a request that has already
> left the queue and is pending. Though, again, this isn't really
> related to this patch unless I'm missing something.

Sorry for the confusion, this patch is actually fine what we really
need to fix is bt_att_cancel should not really remove the pending
request otherwise the remote may disconnect if we proceed with another
request.


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




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux