Hi Gerd, On Thu, Jan 25, 2018 at 6:16 AM, Gerd Hoffmann <kra...@redhat.com> wrote: > From: Mao Zhongyi <maozy.f...@cn.fujitsu.com> > > Replace init() of CCIDCardClass with realize, then convert > ccid_card_init(), ccid_card_initfn() and it's callbacks to > take an Error** in ordor to report the error more clearly. > > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: Marc-André Lureau <marcandre.lur...@gmail.com> > Cc: Cao jin <caoj.f...@cn.fujitsu.com> > > Signed-off-by: Mao Zhongyi <maozy.f...@cn.fujitsu.com> > Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com> > Message-id: > a8b7c670cb61b8096291f5730af62f4230a3e7fa.1514187411.git.maozy.f...@cn.fujitsu.com > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > --- > hw/usb/ccid.h | 2 +- > hw/usb/ccid-card-emulated.c | 44 > +++++++++++++++++++++---------------------- > hw/usb/ccid-card-passthru.c | 12 ++++++------ > hw/usb/dev-smartcard-reader.c | 34 ++++++++++++++++++--------------- > 4 files changed, 48 insertions(+), 44 deletions(-) > > diff --git a/hw/usb/ccid.h b/hw/usb/ccid.h > index 1f070116d6..6c6c10188d 100644 > --- a/hw/usb/ccid.h > +++ b/hw/usb/ccid.h > @@ -34,7 +34,7 @@ typedef struct CCIDCardClass { > const uint8_t *apdu, > uint32_t len); > void (*exitfn)(CCIDCardState *card); > - int (*initfn)(CCIDCardState *card); > + void (*realize)(CCIDCardState *card, Error **errp); > } CCIDCardClass; > > /* > diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c > index e646eb243b..daefd9f8f4 100644 > --- a/hw/usb/ccid-card-emulated.c > +++ b/hw/usb/ccid-card-emulated.c > @@ -35,6 +35,7 @@ > #include "qemu/thread.h" > #include "qemu/main-loop.h" > #include "ccid.h" > +#include "qapi/error.h" > > #define DPRINTF(card, lvl, fmt, ...) \ > do {\ > @@ -401,10 +402,10 @@ static void card_event_handler(EventNotifier *notifier) > qemu_mutex_unlock(&card->event_list_mutex); > } > > -static int init_event_notifier(EmulatedState *card) > +static int init_event_notifier(EmulatedState *card, Error **errp) > { > if (event_notifier_init(&card->notifier, false) < 0) { > - DPRINTF(card, 2, "event notifier creation failed\n"); > + error_setg(errp, "ccid-card-emul: event notifier creation failed"); > return -1; > } > event_notifier_set_handler(&card->notifier, card_event_handler); > @@ -480,7 +481,7 @@ static uint32_t parse_enumeration(char *str, > return ret; > } > > -static int emulated_initfn(CCIDCardState *base) > +static void emulated_realize(CCIDCardState *base, Error **errp) > { > EmulatedState *card = EMULATED_CCID_CARD(base); > VCardEmulError ret; > @@ -494,8 +495,8 @@ static int emulated_initfn(CCIDCardState *base) > qemu_cond_init(&card->handle_apdu_cond); > card->reader = NULL; > card->quit_apdu_thread = 0; > - if (init_event_notifier(card) < 0) { > - return -1; > + if (init_event_notifier(card, errp) < 0) { > + return; > } > > card->backend = 0; > @@ -505,11 +506,11 @@ static int emulated_initfn(CCIDCardState *base) > } > > if (card->backend == 0) { > - printf("backend must be one of:\n"); > + error_setg(errp, "backend must be one of:"); > for (ptable = backend_enum_table; ptable->name != NULL; ++ptable) { > - printf("%s\n", ptable->name); > + error_append_hint(errp, "%s\n", ptable->name); > } > - return -1; > + return; > } > > /* TODO: a passthru backened that works on local machine. third card > type?*/ > @@ -517,34 +518,33 @@ static int emulated_initfn(CCIDCardState *base) > if (card->cert1 != NULL && card->cert2 != NULL && card->cert3 != > NULL) { > ret = emulated_initialize_vcard_from_certificates(card); > } else { > - printf("%s: you must provide all three certs for" > - " certificates backend\n", TYPE_EMULATED_CCID); > - return -1; > + error_setg(errp, "%s: you must provide all three certs for" > + " certificates backend", TYPE_EMULATED_CCID); > + return; > } > } else { > if (card->backend != BACKEND_NSS_EMULATED) { > - printf("%s: bad backend specified. The options are:\n%s > (default)," > - " %s.\n", TYPE_EMULATED_CCID, BACKEND_NSS_EMULATED_NAME, > - BACKEND_CERTIFICATES_NAME); > - return -1; > + error_setg(errp, "%s: bad backend specified. The options are:%s" > + " (default), %s.", TYPE_EMULATED_CCID, > + BACKEND_NSS_EMULATED_NAME, BACKEND_CERTIFICATES_NAME); > + return; > } > if (card->cert1 != NULL || card->cert2 != NULL || card->cert3 != > NULL) { > - printf("%s: unexpected cert parameters to nss emulated > backend\n", > - TYPE_EMULATED_CCID); > - return -1; > + error_setg(errp, "%s: unexpected cert parameters to nss emulated > " > + "backend", TYPE_EMULATED_CCID); > + return; > } > /* default to mirroring the local hardware readers */ > ret = wrap_vcard_emul_init(NULL); > } > if (ret != VCARD_EMUL_OK) { > - printf("%s: failed to initialize vcard\n", TYPE_EMULATED_CCID); > - return -1; > + error_setg(errp, "%s: failed to initialize vcard", > TYPE_EMULATED_CCID); > + return; > } > qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread, > card, QEMU_THREAD_JOINABLE); > qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", > handle_apdu_thread, > card, QEMU_THREAD_JOINABLE); > - return 0; > } > > static void emulated_exitfn(CCIDCardState *base) > @@ -581,7 +581,7 @@ static void emulated_class_initfn(ObjectClass *klass, > void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > CCIDCardClass *cc = CCID_CARD_CLASS(klass); > > - cc->initfn = emulated_initfn; > + cc->realize = emulated_realize; > cc->exitfn = emulated_exitfn; > cc->get_atr = emulated_get_atr; > cc->apdu_from_guest = emulated_apdu_from_guest; > diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c > index 117711862e..b7dd3602dc 100644 > --- a/hw/usb/ccid-card-passthru.c > +++ b/hw/usb/ccid-card-passthru.c > @@ -14,6 +14,7 @@ > #include "qemu/error-report.h" > #include "qemu/sockets.h" > #include "ccid.h" > +#include "qapi/error.h" > > #define DPRINTF(card, lvl, fmt, ...) \ > do { \ > @@ -337,29 +338,28 @@ static const uint8_t *passthru_get_atr(CCIDCardState > *base, uint32_t *len) > return card->atr; > } > > -static int passthru_initfn(CCIDCardState *base) > +static void passthru_realize(CCIDCardState *base, Error **errp) > { > PassthruState *card = PASSTHRU_CCID_CARD(base); > > card->vscard_in_pos = 0; > card->vscard_in_hdr = 0; > if (qemu_chr_fe_backend_connected(&card->cs)) { > - DPRINTF(card, D_INFO, "initing chardev\n"); > + error_setg(errp, "ccid-card-passthru: initing chardev"); > qemu_chr_fe_set_handlers(&card->cs, > ccid_card_vscard_can_read, > ccid_card_vscard_read, > ccid_card_vscard_event, NULL, card, NULL, true); > ccid_card_vscard_send_init(card); > } else { > - error_report("missing chardev"); > - return -1; > + error_setg(errp, "missing chardev"); > + return; > } > card->debug = parse_debug_env("QEMU_CCID_PASSTHRU_DEBUG", D_VERBOSE, > card->debug); > assert(sizeof(DEFAULT_ATR) <= MAX_ATR_SIZE); > memcpy(card->atr, DEFAULT_ATR, sizeof(DEFAULT_ATR)); > card->atr_length = sizeof(DEFAULT_ATR); > - return 0; > } > > static VMStateDescription passthru_vmstate = { > @@ -387,7 +387,7 @@ static void passthru_class_initfn(ObjectClass *klass, > void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > CCIDCardClass *cc = CCID_CARD_CLASS(klass); > > - cc->initfn = passthru_initfn; > + cc->realize = passthru_realize; > cc->get_atr = passthru_get_atr; > cc->apdu_from_guest = passthru_apdu_from_guest; > set_bit(DEVICE_CATEGORY_INPUT, dc->categories); > diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c > index e334d3be11..43c564fe7a 100644 > --- a/hw/usb/dev-smartcard-reader.c > +++ b/hw/usb/dev-smartcard-reader.c > @@ -510,14 +510,18 @@ static void ccid_card_exitfn(CCIDCardState *card) > > } > > -static int ccid_card_initfn(CCIDCardState *card) > +static void ccid_card_initfn(CCIDCardState *card, Error **errp) > { > CCIDCardClass *cc = CCID_CARD_GET_CLASS(card); > + Error *local_err = NULL; > > - if (cc->initfn) { > - return cc->initfn(card); > + if (cc->realize) { > + cc->realize(card, &local_err); > + if (local_err != NULL) { > + error_propagate(errp, local_err); > + return; > + } > } > - return 0; > } > > static bool ccid_has_pending_answers(USBCCIDState *s) > @@ -1295,27 +1299,27 @@ static int ccid_card_exit(DeviceState *qdev) > return 0; > } > > -static int ccid_card_init(DeviceState *qdev) > +static void ccid_card_realize(DeviceState *qdev, Error **errp) > { > CCIDCardState *card = CCID_CARD(qdev); > USBDevice *dev = USB_DEVICE(qdev->parent_bus->parent); > USBCCIDState *s = USB_CCID_DEV(dev); > - int ret = 0; > + Error *local_err = NULL; > > if (card->slot != 0) { > - warn_report("usb-ccid supports one slot, can't add %d", > - card->slot); > - return -1; > + error_setg(errp, "usb-ccid supports one slot, can't add %d", > + card->slot); > + return; > } > if (s->card != NULL) { > - warn_report("usb-ccid card already full, not adding"); > - return -1; > + error_setg(errp, "usb-ccid card already full, not adding"); > + return; > } > - ret = ccid_card_initfn(card); > - if (ret == 0) { > + ccid_card_initfn(card, &local_err); > + if (local_err != NULL) { > + error_propagate(errp, local_err);
noticed while rebasing, is this missing? } else { > s->card = card; > } > - return ret; > } > > static void ccid_realize(USBDevice *dev, Error **errp) > @@ -1477,7 +1481,7 @@ static void ccid_card_class_init(ObjectClass *klass, > void *data) > { > DeviceClass *k = DEVICE_CLASS(klass); > k->bus_type = TYPE_CCID_BUS; > - k->init = ccid_card_init; > + k->realize = ccid_card_realize; > k->exit = ccid_card_exit; > k->props = ccid_props; > } > -- > 2.9.3 > >