[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 10/13] cryptodev: add vhost-user as a new cryptod
From: |
Zhoujian (jay) |
Subject: |
Re: [Qemu-devel] [PULL 10/13] cryptodev: add vhost-user as a new cryptodev backend |
Date: |
Sat, 28 Apr 2018 00:59:12 +0000 |
Hi Peter,
> -----Original Message-----
> From: Peter Maydell [mailto:address@hidden
> Sent: Saturday, April 28, 2018 12:15 AM
> To: Michael S. Tsirkin <address@hidden>
> Cc: QEMU Developers <address@hidden>; Gonglei (Arei)
> <address@hidden>; longpeng <address@hidden>; Zhoujian (jay)
> <address@hidden>; Paolo Bonzini <address@hidden>
> Subject: Re: [PULL 10/13] cryptodev: add vhost-user as a new cryptodev
> backend
>
> On 1 March 2018 at 16:46, Michael S. Tsirkin <address@hidden> wrote:
> > From: Gonglei <address@hidden>
> >
> > Usage:
> > -chardev socket,id=charcrypto0,path=/path/to/your/socket
> > -object cryptodev-vhost-user,id=cryptodev0,chardev=charcrypto0
> > -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0
> >
> > Signed-off-by: Gonglei <address@hidden>
> > Signed-off-by: Longpeng(Mike) <address@hidden>
> > Signed-off-by: Jay Zhou <address@hidden>
> > Reviewed-by: Michael S. Tsirkin <address@hidden>
> > Signed-off-by: Michael S. Tsirkin <address@hidden>
>
> Hi; Coverity (CID 1390600) points out that there's dead code in this function:
>
> > +static void cryptodev_vhost_user_event(void *opaque, int event) {
> > + CryptoDevBackendVhostUser *s = opaque;
> > + CryptoDevBackend *b = CRYPTODEV_BACKEND(s);
> > + Error *err = NULL;
>
> We set err to NULL here...
>
> > + int queues = b->conf.peers.queues;
> > +
> > + assert(queues < MAX_CRYPTO_QUEUE_NUM);
> > +
> > + switch (event) {
> > + case CHR_EVENT_OPENED:
> > + if (cryptodev_vhost_user_start(queues, s) < 0) {
> > + exit(1);
> > + }
> > + b->ready = true;
> > + break;
> > + case CHR_EVENT_CLOSED:
> > + b->ready = false;
> > + cryptodev_vhost_user_stop(queues, s);
> > + break;
> > + }
>
> ...and nothing here does anything with err...
>
> > +
> > + if (err) {
> > + error_report_err(err);
> > + }
>
> ...so this if() is all dead code and we could remove err entirely.
Thanks for reporting this. I'll send a patch to fix it.
Regards,
Jay