[Spice-devel] [PATCH 9/9] client/smartcard: libcacard dropped ReaderAddResponse
Alon Levy
alevy at redhat.com
Wed Feb 9 02:42:09 PST 2011
On Wed, Feb 09, 2011 at 11:50:23AM +0100, Hans de Goede wrote:
> Hi,
>
> On second look, my main comment was invalid, so Ack.
>
> I still think that VSC_Error should be renamed to
> VSC_Status and like wise the new handle_error function
> should be renamed to handle_status. But that can be done in
> a separate patch. Or you can disagree and just leave it as is :)
>
Yeah, I agree about the names, will do it later (as long as I
don't change VSC_Status changing the handler's name will just
cause confusion, mainly to me when I come back to this code
later, so better do them at the same time - the first requires
changing the protocol, even though it's just a rename, in
libcacard. argh..)
> Regards,
>
> Hans
>
>
> On 02/09/2011 11:20 AM, Alon Levy wrote:
> >uses VSC_Error with code==VSC_SUCCESS instead. This means that the VSC_Error
> >message is overloaded. Instead of the other option of adding a message id,
> >since the connection is TCP so no messages may be dropped or reordered, by
> >having each message followed by a response there is no ambiguity. Still
> >this commit adds a queue for messages that we only have one of which outstanding
> >at a time, i.e. send, wait for response, send the next, etc. This further
> >simplifies the logic, while not adding much overhead since only when spicec
> >starts up it has a situation where it needs to send two events (ReaderAdd
> >and ATR for Card Insert).
> >---
> > client/smartcard_channel.cpp | 87 +++++++++++++++++++++++++++++++++---------
> > client/smartcard_channel.h | 9 ++++
> > 2 files changed, 78 insertions(+), 18 deletions(-)
> >
> >diff --git a/client/smartcard_channel.cpp b/client/smartcard_channel.cpp
> >index c57ecd5..98f24a8 100644
> >--- a/client/smartcard_channel.cpp
> >+++ b/client/smartcard_channel.cpp
> >@@ -45,6 +45,7 @@ public:
> >
> > SmartCardChannel::SmartCardChannel(RedClient& client, uint32_t id)
> > : RedChannel(client, SPICE_CHANNEL_SMARTCARD, id, new SmartCardHandler(*this))
> >+ , _next_sync_vevent(VEVENT_LAST)
> > {
> > SmartCardHandler* handler = static_cast<SmartCardHandler*>(get_message_handler());
> >
> >@@ -84,6 +85,7 @@ void SmartCardChannel::add_unallocated_reader(VReader* vreader, const char* name
> > data->reader_id = VSCARD_UNDEFINED_READER_ID;
> > data->name = spice_strdup(name);
> > _unallocated_readers_by_vreader.insert(std::pair<VReader*, ReaderData*>(vreader, data));
> >+ LOG_INFO("adding unallocated reader %p", data);
> > }
> >
> > /** called upon the VSC_ReaderAddResponse
> >@@ -96,6 +98,7 @@ ReaderData* SmartCardChannel::add_reader(uint32_t reader_id)
> > assert(unallocated> 0);
> > data = _unallocated_readers_by_vreader.begin()->second;
> > data->reader_id = reader_id;
> >+ LOG_INFO("adding %p->%d", data, reader_id);
> > _readers_by_vreader.insert(
> > std::pair<VReader*, ReaderData*>(data->vreader, data));
> > assert(_readers_by_vreader.count(data->vreader) == 1);
> >@@ -179,6 +182,65 @@ void SmartCardChannel::remove_reader(ReaderData* data)
> > delete data;
> > }
> >
> >+/* Sync events need to be sent one by one, waiting for VSC_Error
> >+ * messages from the server in between. */
> >+void SmartCardChannel::push_sync_event(VEventType type, Event *event)
> >+{
> >+ event->ref();
> >+ _sync_events.push_back(SmartCardEvent(type, event));
> >+ if (_next_sync_vevent != VEVENT_LAST) {
> >+ return;
> >+ }
> >+ send_next_sync_event();
> >+}
> >+
> >+void SmartCardChannel::send_next_sync_event()
> >+{
> >+ if (_sync_events.empty()) {
> >+ _next_sync_vevent = VEVENT_LAST;
> >+ return;
> >+ }
> >+ SmartCardEvent sync_event = _sync_events.front();
> >+ _sync_events.pop_front();
> >+ get_client().push_event(sync_event.second);
> >+ sync_event.second->unref();
> >+ _next_sync_vevent = sync_event.first;
> >+}
> >+
> >+void SmartCardChannel::handle_reader_add_response(VSCMsgHeader *vheader,
> >+ VSCMsgError *error)
> >+{
> >+ ReaderData* data;
> >+
> >+ if (error->code == VSC_SUCCESS) {
> >+ data = add_reader(vheader->reader_id);
> >+ if (data->card_insert_pending) {
> >+ data->card_insert_pending = false;
> >+ send_atr(data->vreader);
> >+ }
> >+ } else {
> >+ LOG_WARN("VSC Error: reader %d, code %d",
> >+ vheader->reader_id, error->code);
> >+ }
> >+}
> >+
> >+void SmartCardChannel::handle_error_message(VSCMsgHeader *vheader,
> >+ VSCMsgError *error)
> >+{
> >+ switch (_next_sync_vevent) {
> >+ case VEVENT_READER_INSERT:
> >+ handle_reader_add_response(vheader, error);
> >+ break;
> >+ case VEVENT_CARD_INSERT:
> >+ case VEVENT_CARD_REMOVE:
> >+ case VEVENT_READER_REMOVE:
> >+ break;
> >+ default:
> >+ LOG_WARN("Unexpected Error message: %d", error->code);
> >+ }
> >+ send_next_sync_event();
> >+}
> >+
> > void SmartCardChannel::cac_card_events_thread_main()
> > {
> > VEvent *vevent = NULL;
> >@@ -194,28 +256,28 @@ void SmartCardChannel::cac_card_events_thread_main()
> > LOG_INFO("VEVENT_READER_INSERT");
> > {
> > AutoRef<ReaderAddEvent> event(new ReaderAddEvent(this, vevent));
> >- get_client().push_event(*event);
> >+ push_sync_event(vevent->type, *event);
> > }
> > break;
> > case VEVENT_READER_REMOVE:
> > LOG_INFO("VEVENT_READER_REMOVE");
> > {
> > AutoRef<ReaderRemoveEvent> event(new ReaderRemoveEvent(this, vevent));
> >- get_client().push_event(*event);
> >+ push_sync_event(vevent->type, *event);
> > }
> > break;
> > case VEVENT_CARD_INSERT:
> > LOG_INFO("VEVENT_CARD_INSERT");
> > {
> > AutoRef<CardInsertEvent> event(new CardInsertEvent(this, vevent));
> >- get_client().push_event(*event);
> >+ push_sync_event(vevent->type, *event);
> > }
> > break;
> > case VEVENT_CARD_REMOVE:
> > LOG_INFO("VEVENT_CARD_REMOVE");
> > {
> > AutoRef<CardRemoveEvent> event(new CardRemoveEvent(this, vevent));
> >- get_client().push_event(*event);
> >+ push_sync_event(vevent->type, *event);
> > }
> > break;
> > case VEVENT_LAST:
> >@@ -390,25 +452,14 @@ void VSCMessageEvent::response(AbstractProcessLoop& loop)
> > uint8_t pbRecvBuffer[APDUBufSize+sizeof(uint32_t)];
> > VReaderStatus reader_status;
> > uint32_t rv;
> >- ReaderData* data;
> >
> > switch (_vheader->type) {
> >- case (VSC_ReaderAddResponse):
> >- data = _smartcard_channel->add_reader(_vheader->reader_id);
> >- if (data->card_insert_pending) {
> >- data->card_insert_pending = false;
> >- _smartcard_channel->send_atr(data->vreader);
> >- }
> >- return;
> >- break;
> > case VSC_APDU:
> > break;
> > case VSC_Error:
> >- {
> >- VSCMsgError *error = (VSCMsgError*)_vheader->data;
> >- LOG_WARN("VSC Error: reader %d, code %d",
> >- _vheader->reader_id, error->code);
> >- }
> >+ _smartcard_channel->handle_error_message(
> >+ _vheader,
> >+ (VSCMsgError*)_vheader->data);
> > return;
> > case VSC_Init:
> > break;
> >diff --git a/client/smartcard_channel.h b/client/smartcard_channel.h
> >index 1d512a6..752713b 100644
> >--- a/client/smartcard_channel.h
> >+++ b/client/smartcard_channel.h
> >@@ -86,6 +86,8 @@ public:
> > virtual void response(AbstractProcessLoop& events_loop);
> > };
> >
> >+typedef std::pair<VEventType, Event*> SmartCardEvent;
> >+
> > class SmartCardChannel : public RedChannel {
> >
> > public:
> >@@ -114,6 +116,13 @@ private:
> > typedef std::map<VReader*, ReaderData*> readers_by_vreader_t;
> > readers_by_vreader_t _readers_by_vreader;
> > readers_by_vreader_t _unallocated_readers_by_vreader;
> >+ VEventType _next_sync_vevent;
> >+ std::list<SmartCardEvent> _sync_events;
> >+
> >+ void push_sync_event(VEventType type, Event *event);
> >+ void send_next_sync_event();
> >+ void handle_reader_add_response(VSCMsgHeader *vheader, VSCMsgError *error);
> >+ void handle_error_message(VSCMsgHeader *vheader, VSCMsgError *error);
> >
> > ReaderData* reader_data_from_vreader(VReader* vreader);
> > ReaderData* reader_data_from_reader_id(uint32_t reader_id);
More information about the Spice-devel
mailing list