[Spice-devel] [PATCH 4/8] client/smartcard: libcacard dropped ReaderAddResponse

[Hans de Goede]

Hi,

On 02/04/2011 09:39 AM, Alon Levy wrote:

<snip>

>> Hmm,
>>
>> This seems error prone. What if the other side is sending a success state for some
>> other reason? Either error->code == VSC_SUCCESS can only be send for the
>> adding a reader case and keeping track of num_pending_reader_inserts is
>> not necessary or this seems racy to me. Maybe add a VCS_READER_ADD_SUCCESS
>> error code ?
>
> Ok, I admit this whole counter was a way to quickly fix the problem that I was getting a second VSC_SUCCESS. The idea was to have a single outstanding request that needs an answer, thereby removing ambiguity. But the implementation doesn't do that, hence the hack of _num_pending_reader_inserts. To keep the protocol as is (btw I just *removed* such a message because I wanted to simplify the protocol - ReaderAddResponse. equivalent to the suggested error code) I could just serialize to:
>   client: request reader add
>   client: wait for response
>   server: send VSCError.code==VSC_SUCCESS
>   client: report card insert
>   cient:  wait for response
>

That is still racy. What if a VSCError.code==VSC_SUCCESS is already in the pipe
from server to client for some other request send by the client earlier. Then
the client will report the card insert, when it receives the unrelated
VSCError.code==VSC_SUCCESS.

The way I've fixed issues like this with the usb network redirection stuff
I'm doing is give the common header for all requests an id field and increment
that which each packet send. When the server (in your case) then sends a response
that includes the id of the request it is a response to so that requests and
responses can be mapped 1 on 1.

Regards,

Hans