[Spice-devel] [PATCH 9/9] client/smartcard: libcacard dropped ReaderAddResponse
Hans de Goede
hdegoede at redhat.com
Wed Feb 9 02:50:23 PST 2011
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 :)
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