> > On Fri, 2016-09-30 at 14:21 +0100, Frediano Ziglio wrote: > > --- > > server/Makefile.am | 2 + > > server/smartcard-channel-client.c | 300 > > +++++++++++++++++++++++++++++++ > > server/smartcard-channel-client.h | 96 ++++++++++ > > server/smartcard.c | 360 ++++++-------------------- > > ------------ > > server/smartcard.h | 21 +++ > > 5 files changed, 471 insertions(+), 308 deletions(-) > > create mode 100644 server/smartcard-channel-client.c > > create mode 100644 server/smartcard-channel-client.h > > > > diff --git a/server/Makefile.am b/server/Makefile.am > > index 9c5b3b6..abbec16 100644 > > --- a/server/Makefile.am > > +++ b/server/Makefile.am > > @@ -172,6 +172,8 @@ if HAVE_SMARTCARD > > libserver_la_SOURCES += \ > > smartcard.c \ > > smartcard.h \ > > + smartcard-channel-client.c \ > > + smartcard-channel-client.h \ > > $(NULL) > > endif > > > > diff --git a/server/smartcard-channel-client.c b/server/smartcard- > > channel-client.c > > new file mode 100644 > > index 0000000..0622be5 > > --- /dev/null > > +++ b/server/smartcard-channel-client.c > > @@ -0,0 +1,300 @@ > > +/* > > + Copyright (C) 2009-2015 Red Hat, Inc. > > + > > + This library is free software; you can redistribute it and/or > > + modify it under the terms of the GNU Lesser General Public > > + License as published by the Free Software Foundation; either > > + version 2.1 of the License, or (at your option) any later > > version. > > + > > + This library is distributed in the hope that it will be useful, > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + Lesser General Public License for more details. > > + > > + You should have received a copy of the GNU Lesser General Public > > + License along with this library; if not, see <http://www.gnu.org/ > > licenses/>. > > +*/ > > +#ifdef HAVE_CONFIG_H > > +#include <config.h> > > +#endif > > + > > +#include "smartcard-channel-client.h" > > + > > +typedef struct RedErrorItem { > > + RedPipeItem base; > > + VSCMsgHeader vheader; > > + VSCMsgError error; > > +} RedErrorItem; > > + > > +uint8_t *smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient > > *rcc, > > + uint16_t type, > > + uint32_t size) > > +{ > > + SmartCardChannelClient *scc = SMARTCARD_CHANNEL_CLIENT(rcc); > > + RedClient *client = red_channel_client_get_client(rcc); > > + > > + /* todo: only one reader is actually supported. When we fix the > > code to support > > + * multiple readers, we will porbably associate different > > devices to > > + * differenc channels */ > > > Hmm, just noticed these typos. Should we fix them in this commit? > "porbably" > "differenc" > Can go in this or a following patch. > > > + if (!scc->priv->smartcard) { > > + scc->priv->msg_in_write_buf = FALSE; > > + return spice_malloc(size); > > + } else { > > + RedCharDeviceSmartcard *smartcard; > > + > > + spice_assert(smartcard_get_n_readers() == 1); > > + smartcard = scc->priv->smartcard; > > + spice_assert(smartcard_char_device_get_client(smartcard) || > > scc->priv->smartcard); > > + spice_assert(!scc->priv->write_buf); > > + scc->priv->write_buf = > > + red_char_device_write_buffer_get(RED_CHAR_DEVICE(smartca > > rd), client, > > + size); > > + > > + if (!scc->priv->write_buf) { > > + spice_error("failed to allocate write buffer"); > > + return NULL; > > + } > > + scc->priv->msg_in_write_buf = TRUE; > > + return scc->priv->write_buf->buf; > > + } > > +} > > + > > +void smartcard_channel_client_release_msg_rcv_buf(RedChannelClient > > *rcc, > > + uint16_t type, > > + uint32_t size, > > + uint8_t *msg) > > +{ > > + SmartCardChannelClient *scc = SMARTCARD_CHANNEL_CLIENT(rcc); > > + > > + /* todo: only one reader is actually supported. When we fix the > > code to support > > + * multiple readers, we will porbably associate different > > devices to > > + * differenc channels */ > > Same typos here. > > > > + > > + if (!scc->priv->msg_in_write_buf) { > > + spice_assert(!scc->priv->write_buf); > > + free(msg); > > + } else { > > + if (scc->priv->write_buf) { /* msg hasn't been pushed to the > > guest */ > > + spice_assert(scc->priv->write_buf->buf == msg); > > + red_char_device_write_buffer_release(RED_CHAR_DEVICE(scc > > ->priv->smartcard), > > + &scc->priv- > > >write_buf); > > + } > > + } > > +} > > + > > +void smartcard_channel_client_on_disconnect(RedChannelClient *rcc) > > +{ > > + SmartCardChannelClient *scc = SMARTCARD_CHANNEL_CLIENT(rcc); > > + RedCharDeviceSmartcard *device = scc->priv->smartcard; > > + > > + if (device) { > > + smartcard_char_device_detach_client(device, scc); > > + smartcard_char_device_notify_reader_remove(device); > > + } > > +} > > + > > +void smartcard_channel_client_send_data(RedChannelClient *rcc, > > + SpiceMarshaller *m, > > + RedPipeItem *item, > > + VSCMsgHeader *vheader) > > +{ > > + spice_assert(rcc); > > + spice_assert(vheader); > > + red_channel_client_init_send_data(rcc, SPICE_MSG_SMARTCARD_DATA, > > item); > > + spice_marshaller_add_ref(m, (uint8_t*)vheader, > > sizeof(VSCMsgHeader)); > > + if (vheader->length > 0) { > > + spice_marshaller_add_ref(m, (uint8_t*)(vheader+1), vheader- > > >length); > > + } > > +} > > + > > +void smartcard_channel_client_send_error(RedChannelClient *rcc, > > SpiceMarshaller *m, RedPipeItem *item) > > +{ > > + RedErrorItem* error_item = SPICE_UPCAST(RedErrorItem, item); > > + > > + smartcard_channel_client_send_data(rcc, m, item, &error_item- > > >vheader); > > +} > > + > > +static void smartcard_channel_client_push_error(RedChannelClient > > *rcc, > > + uint32_t reader_id, > > + VSCErrorCode error) > > +{ > > + RedErrorItem *error_item = spice_new0(RedErrorItem, 1); > > + > > + red_pipe_item_init(&error_item->base, RED_PIPE_ITEM_TYPE_ERROR); > > + > > + error_item->vheader.reader_id = reader_id; > > + error_item->vheader.type = VSC_Error; > > + error_item->vheader.length = sizeof(error_item->error); > > + error_item->error.code = error; > > + red_channel_client_pipe_add_push(rcc, &error_item->base); > > +} > > + > > +static void > > smartcard_channel_client_add_reader(SmartCardChannelClient *scc, > > + uint8_t *name) > > +{ > > + if (!scc->priv->smartcard) { /* we already tried to attach a > > reader to the client > > + when it connected */ > > This comment has odd indentation. > > > + SpiceCharDeviceInstance *char_device = > > smartcard_readers_get_unattached(); > > + > > + if (!char_device) { > > + smartcard_channel_client_push_error(RED_CHANNEL_CLIENT(s > > cc), > > + VSCARD_UNDEFINED_REA > > DER_ID, > > + VSC_CANNOT_ADD_MORE_ > > READERS); > > + return; > > + } > > + smartcard_char_device_attach_client(char_device, scc); > > + } > > + smartcard_char_device_notify_reader_add(scc->priv->smartcard); > > + // The device sends a VSC_Error message, we will let it through, > > no > > + // need to send our own. We already set the correct reader_id, > > from > > + // our RedCharDeviceSmartcard. > > +} > > + > > +static void > > smartcard_channel_client_remove_reader(SmartCardChannelClient *scc, > > + uint32_t > > reader_id) > > +{ > > + SpiceCharDeviceInstance *char_device = > > smartcard_readers_get(reader_id); > > + RedCharDeviceSmartcard *dev; > > + > > + if (char_device == NULL) { > > + smartcard_channel_client_push_error(RED_CHANNEL_CLIENT(scc), > > + reader_id, > > VSC_GENERAL_ERROR); > > + return; > > + } > > + > > + dev = red_char_device_opaque_get(char_device->st); > > + spice_assert(scc->priv->smartcard == dev); > > + if (!smartcard_char_device_notify_reader_remove(dev)) { > > + smartcard_channel_client_push_error(RED_CHANNEL_CLIENT(scc), > > + reader_id, > > VSC_GENERAL_ERROR); > > + return; > > + } > > +} > > + > > +RedCharDeviceSmartcard* > > smartcard_channel_client_get_device(SmartCardChannelClient *scc) > > +{ > > + return scc->priv->smartcard; > > +} > > + > > +static void > > smartcard_channel_client_write_to_reader(SmartCardChannelClient *scc) > > +{ > > + g_return_if_fail(scc); > > + > > + smartcard_channel_write_to_reader(scc->priv->write_buf); > > + scc->priv->write_buf = NULL; > > +} > > + > > + > > +int smartcard_channel_client_handle_message(RedChannelClient *rcc, > > + uint16_t type, > > + uint32_t size, > > + uint8_t *msg) > > +{ > > + VSCMsgHeader* vheader = (VSCMsgHeader*)msg; > > + SmartCardChannelClient *scc = SMARTCARD_CHANNEL_CLIENT(rcc); > > + > > + if (type != SPICE_MSGC_SMARTCARD_DATA) { > > + /* Handles seamless migration protocol. Also handles ack's, > > + * spicy sends them while spicec does not */ > > + return red_channel_client_handle_message(rcc, size, type, > > msg); > > + } > > + > > + spice_assert(size == vheader->length + sizeof(VSCMsgHeader)); > > + switch (vheader->type) { > > + case VSC_ReaderAdd: > > + smartcard_channel_client_add_reader(scc, msg + > > sizeof(VSCMsgHeader)); > > + return TRUE; > > + break; > > + case VSC_ReaderRemove: > > + smartcard_channel_client_remove_reader(scc, vheader- > > >reader_id); > > + return TRUE; > > + break; > > + case VSC_Init: > > + // ignore - we should never get this anyway > > + return TRUE; > > + break; > > + case VSC_Error: > > + case VSC_ATR: > > + case VSC_CardRemove: > > + case VSC_APDU: > > + break; // passed on to device > > + default: > > + printf("ERROR: unexpected message on smartcard > > channel\n"); > > + return TRUE; > > + } > > + > > + /* todo: fix */ > > + if (vheader->reader_id >= smartcard_get_n_readers()) { > > + spice_printerr("ERROR: received message for non existing > > reader: %d, %d, %d", vheader->reader_id, > > + vheader->type, vheader->length); > > + return FALSE; > > + } > > + spice_assert(scc->priv->write_buf->buf == msg); > > + smartcard_channel_client_write_to_reader(scc); > > + > > + return TRUE; > > +} > > + > > +int smartcard_channel_client_handle_migrate_data(RedChannelClient > > *rcc, > > + uint32_t size, > > + void *message) > > +{ > > + SmartCardChannelClient *scc; > > + SpiceMigrateDataHeader *header; > > + SpiceMigrateDataSmartcard *mig_data; > > + > > + scc = SMARTCARD_CHANNEL_CLIENT(rcc); > > + header = (SpiceMigrateDataHeader *)message; > > + mig_data = (SpiceMigrateDataSmartcard *)(header + 1); > > + if (size < sizeof(SpiceMigrateDataHeader) + > > sizeof(SpiceMigrateDataSmartcard)) { > > + spice_error("bad message size"); > > + return FALSE; > > + } > > + if (!migration_protocol_validate_header(header, > > + SPICE_MIGRATE_DATA_SMART > > CARD_MAGIC, > > + SPICE_MIGRATE_DATA_SMART > > CARD_VERSION)) { > > + spice_error("bad header"); > > + return FALSE; > > + } > > + > > + if (!mig_data->base.connected) { /* client wasn't attached to a > > smartcard */ > > + return TRUE; > > + } > > + > > + if (!scc->priv->smartcard) { > > + SpiceCharDeviceInstance *char_device = > > smartcard_readers_get_unattached(); > > + > > + if (!char_device) { > > + spice_warning("no unattached device available"); > > + return TRUE; > > + } else { > > + smartcard_char_device_attach_client(char_device, scc); > > + } > > + } > > + spice_debug("reader added %d partial read_size %u", mig_data- > > >reader_added, mig_data->read_size); > > + > > + return smartcard_char_device_handle_migrate_data(scc->priv- > > >smartcard, > > + mig_data); > > +} > > + > > +int > > smartcard_channel_client_handle_migrate_flush_mark(RedChannelClient > > *rcc) > > +{ > > + red_channel_client_pipe_add_type(rcc, > > RED_PIPE_ITEM_TYPE_SMARTCARD_MIGRATE_DATA); > > + return TRUE; > > +} > > + > > +void smartcard_channel_client_set_char_device(SmartCardChannelClient > > *scc, > > + RedCharDeviceSmartcard > > *device) > > +{ > > + if (device == scc->priv->smartcard) { > > + return; > > + } > > + > > + scc->priv->smartcard = device; > > +} > > + > > +RedCharDeviceSmartcard* > > smartcard_channel_client_get_char_device(SmartCardChannelClient *scc) > > +{ > > + return scc->priv->smartcard; > > +} > > + > > diff --git a/server/smartcard-channel-client.h b/server/smartcard- > > channel-client.h > > new file mode 100644 > > index 0000000..44a966a > > --- /dev/null > > +++ b/server/smartcard-channel-client.h > > @@ -0,0 +1,96 @@ > > +/* > > + Copyright (C) 2009-2015 Red Hat, Inc. > > + > > + This library is free software; you can redistribute it and/or > > + modify it under the terms of the GNU Lesser General Public > > + License as published by the Free Software Foundation; either > > + version 2.1 of the License, or (at your option) any later > > version. > > + > > + This library is distributed in the hope that it will be useful, > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + Lesser General Public License for more details. > > + > > + You should have received a copy of the GNU Lesser General Public > > + License along with this library; if not, see <http://www.gnu.org/ > > licenses/>. > > +*/ > > + > > +#ifndef SMARTCARD_CHANNEL_CLIENT_H__ > > +#define SMARTCARD_CHANNEL_CLIENT_H__ > > + > > +#include "smartcard.h" > > +#include "red-channel-client.h" > > + > > +typedef struct SmartCardChannelClientPrivate > > SmartCardChannelClientPrivate; > > +struct SmartCardChannelClientPrivate { > > + RedCharDeviceSmartcard *smartcard; > > + > > + /* read_from_client/write_to_device buffer. > > + * The beginning of the buffer should always be VSCMsgHeader*/ > > + RedCharDeviceWriteBuffer *write_buf; > > + int msg_in_write_buf; /* was the client msg received into a > > RedCharDeviceWriteBuffer > > + * or was it explicitly malloced */ > > +}; > > + > > +typedef struct SmartCardChannelClient { > > + RedChannelClient base; > > + > > + SmartCardChannelClientPrivate priv[1]; > > +} SmartCardChannelClient; > > + > > +#define SMARTCARD_CHANNEL_CLIENT(rcc) ((SmartCardChannelClient*)rcc) > > + > > +SmartCardChannelClient* smartcard_channel_client_create(RedChannel > > *channel, > > + RedClient > > *client, RedsStream *stream, > > + int > > monitor_latency, > > + int > > num_common_caps, uint32_t *common_caps, > > + int > > num_caps, uint32_t *caps); > > + > > +uint8_t* smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient > > *rcc, > > + uint16_t type, > > + uint32_t size); > > + > > +void smartcard_channel_client_release_msg_rcv_buf(RedChannelClient > > *rcc, > > + uint16_t type, > > + uint32_t size, > > + uint8_t *msg); > > + > > +int > > smartcard_channel_client_handle_migrate_flush_mark(RedChannelClient > > *rcc); > > + > > +void smartcard_channel_client_on_disconnect(RedChannelClient *rcc); > > + > > +void smartcard_channel_client_send_data(RedChannelClient *rcc, > > + SpiceMarshaller *m, > > + RedPipeItem *item, > > + VSCMsgHeader *vheader); > > + > > +void smartcard_channel_client_send_error(RedChannelClient *rcc, > > + SpiceMarshaller *m, > > + RedPipeItem *item); > > + > > +RedCharDeviceSmartcard* > > smartcard_channel_client_get_device(SmartCardChannelClient *scc); > > + > > +int smartcard_channel_client_handle_message(RedChannelClient *rcc, > > + uint16_t type, > > + uint32_t size, > > + uint8_t *msg); > > + > > +int smartcard_channel_client_handle_migrate_data(RedChannelClient > > *rcc, > > + uint32_t size, > > + void *message); > > + > > +void smartcard_channel_client_set_char_device(SmartCardChannelClient > > *scc, > > + RedCharDeviceSmartcard > > *device); > > + > > +RedCharDeviceSmartcard* > > smartcard_channel_client_get_char_device(SmartCardChannelClient > > *scc); > > + > > +void smartcard_channel_client_release_msg_rcv_buf(RedChannelClient > > *rcc, > > + uint16_t type, > > + uint32_t size, > > + uint8_t *msg); > > + > > +uint8_t *smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient > > *rcc, > > + uint16_t type, > > + uint32_t size); > > + > > +#endif /* SMARTCARD_CHANNEL_CLIENT_H__ */ > > diff --git a/server/smartcard.c b/server/smartcard.c > > index 41dc106..ab95260 100644 > > --- a/server/smartcard.c > > +++ b/server/smartcard.c > > @@ -30,8 +30,8 @@ > > > > #include "reds.h" > > #include "char-device.h" > > -#include "red-channel-client.h" > > #include "smartcard.h" > > +#include "smartcard-channel-client.h" > > #include "migration-protocol.h" > > > > /* > > @@ -49,25 +49,6 @@ > > // Maximal length of APDU > > #define APDUBufSize 270 > > > > -typedef struct SmartCardChannelClientPrivate > > SmartCardChannelClientPrivate; > > -struct SmartCardChannelClientPrivate { > > - RedCharDeviceSmartcard *smartcard; > > - > > - /* read_from_client/write_to_device buffer. > > - * The beginning of the buffer should always be VSCMsgHeader*/ > > - RedCharDeviceWriteBuffer *write_buf; > > - int msg_in_write_buf; /* was the client msg received into a > > RedCharDeviceWriteBuffer > > - * or was it explicitly malloced */ > > -}; > > - > > -typedef struct SmartCardChannelClient { > > - RedChannelClient base; > > - > > - SmartCardChannelClientPrivate priv[1]; > > -} SmartCardChannelClient; > > - > > -#define SMARTCARD_CHANNEL_CLIENT(rcc) ((SmartCardChannelClient*)rcc) > > - > > G_DEFINE_TYPE(RedCharDeviceSmartcard, red_char_device_smartcard, > > RED_TYPE_CHAR_DEVICE) > > > > #define RED_CHAR_DEVICE_SMARTCARD_PRIVATE(o) > > (G_TYPE_INSTANCE_GET_PRIVATE ((o), RED_TYPE_CHAR_DEVICE_SMARTCARD, > > RedCharDeviceSmartcardPrivate)) > > @@ -84,18 +65,6 @@ struct RedCharDeviceSmartcardPrivate { > > int reader_added; // has reader_add been sent > > to the device > > }; > > > > -enum { > > - RED_PIPE_ITEM_TYPE_ERROR = RED_PIPE_ITEM_TYPE_CHANNEL_BASE, > > - RED_PIPE_ITEM_TYPE_SMARTCARD_DATA, > > - RED_PIPE_ITEM_TYPE_SMARTCARD_MIGRATE_DATA, > > -}; > > - > > -typedef struct RedErrorItem { > > - RedPipeItem base; > > - VSCMsgHeader vheader; > > - VSCMsgError error; > > -} RedErrorItem; > > - > > typedef struct RedMsgItem { > > RedPipeItem base; > > > > @@ -114,12 +83,7 @@ static struct Readers { > > SpiceCharDeviceInstance* sin[SMARTCARD_MAX_READERS]; > > } g_smartcard_readers = {0, {NULL}}; > > > > -static SpiceCharDeviceInstance* > > smartcard_readers_get_unattached(void); > > -static SpiceCharDeviceInstance* smartcard_readers_get(uint32_t > > reader_id); > > static int smartcard_char_device_add_to_readers(RedsState *reds, > > SpiceCharDeviceInstance *sin); > > -static void smartcard_char_device_attach_client( > > - SpiceCharDeviceInstance *char_device, SmartCardChannelClient > > *scc); > > -static void > > smartcard_channel_write_to_reader(RedCharDeviceWriteBuffer > > *write_buf); > > > > static RedMsgItem *smartcard_char_device_on_message_from_device( > > RedCharDeviceSmartcard *dev, VSCMsgHeader *header); > > @@ -179,11 +143,12 @@ static void > > smartcard_send_msg_to_client(RedPipeItem *msg, > > void *opaque) > > { > > RedCharDeviceSmartcard *dev = opaque; > > + RedChannelClient *rcc = RED_CHANNEL_CLIENT(dev->priv->scc); > > > > spice_assert(dev->priv->scc && > > - red_channel_client_get_client(&dev->priv->scc- > > >base) == client); > > + red_channel_client_get_client(rcc) == client); > > red_pipe_item_ref(msg); > > - smartcard_channel_client_pipe_add_push(&dev->priv->scc->base, > > msg); > > + smartcard_channel_client_pipe_add_push(rcc, msg); > > > Perhaps we should just change this to > red_channel_client_pipe_add_push() and remove the smartcard_ version. > Yes, make not much sense. Not a regression but the red_pipe_item_ref before is weird. > ... > > + RED_PIPE_ITEM_TYPE_SMARTCARD_MIGRATE_DATA, > > +}; > > > > #endif // __SMART_CARD_H__ > > > Otherwise looks fine. > Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> > Acked too. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel