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

[Alon Levy]

On Fri, Feb 04, 2011 at 10:11:59AM +0100, Hans de Goede wrote:
> 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 connection is tcp, no drops, so if the client keeps track correctly it
can't happen. put another way, assuming you are correct then there was a message
earlier that the client sent and it assumed was responded to => client bug.
q.e.d.
> 
> 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

Yes, of course that's the canonical fix, also used in spice protocol as you know,
I was trying to avoid that because, well, it adds code, but perhaps you are right..

> 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
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel