qemu-devel
[Top][All Lists]
Advanced

[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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]