Hi Hendrik,
On Fri, Jan 13, 2012, Hendrik Sattler wrote:
>>>> opcode = obex->rx_last_op;
>>>> /* Unexpected response -- fail silently */
>>>>- if (opcode > 0x1f && opcode < 0xff) {
>>>>+ if (opcode > G_OBEX_OP_ABORT && opcode < 0xff) {
>>>> obex->rx_data = 0;
>>>> return TRUE;
>>>> }
>>>
>>>This one always evaluates to false because 0xff == G_OBEX_OP_ABORT |
>>>FINAL_BIT.
>>
>> The value of obex->rx_last_op is stored with the final bit cleared,
>> so the < 0xff part could also be removed from the test. The value
>> 0x1f has been picked because the IrDA OBEX specification defines 0x10
>> - 0x1f as a user-definable range, so anything between 0x1f and 0x7f
>> isn't actually valid. The correct test would then become:
>>
>> if (opcode > 0x1f && opcode != G_OBEX_OP_ABORT)
>>
>> Jaganath, with the above change the patch should be ok, but the more
>> surprising thing to me here is that this implies we're missing one or
>> more unit tests for Abort. Could you create a patch to add those too?
>> (if we're really strict those tests should go in before this patch so
>> that it's actually possible to see that the patch makes a
>> difference).
>
> User-definable does not mean invalid. It only means that these can be
> used for custom commands. These are still bound to the rules of the
> OBEX protocol. I've never seen one using that, though.
Exactly. You might want to re-read what I said and the test I proposed.
> Else you'd also have to filter 0x04 and the range 0x08-0x0f because
> these are marked as "reserved".
With the test I proposed we filter neither reserved (since they might
get meaning in new spec versions) nor user-defined opcodes.
Johan
--
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
