On Mon, Apr 08, 2019 at 11:31:11AM +0800, Tiwei Bie wrote:
> On Fri, Apr 05, 2019 at 03:37:06PM +0100, Bruce Richardson wrote:
> > Coverity points out that there is a check in the main thread loop for the
> > ctrlr->bdev being NULL, but by that stage the pointer has already been
> > dereferenced. Therefore, for safety, before we enter the loop do an
> > initial check on the parameter structure.
> >
> > Coverity issue: 158657
> > Fixes: db75c7af19bb ("examples/vhost_scsi: introduce a new sample app")
> > CC: sta...@dpdk.org
> > CC: Maxime Coquelin <maxime.coque...@redhat.com>
> > CC: Tiwei Bie <tiwei....@intel.com>
> > CC: Zhihong Wang <zhihong.w...@intel.com>
> >
> > Signed-off-by: Bruce Richardson <bruce.richard...@intel.com>
> > ---
> > examples/vhost_scsi/vhost_scsi.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/examples/vhost_scsi/vhost_scsi.c
> > b/examples/vhost_scsi/vhost_scsi.c
> > index 2908ff68b..a6465d089 100644
> > --- a/examples/vhost_scsi/vhost_scsi.c
> > +++ b/examples/vhost_scsi/vhost_scsi.c
> > @@ -285,6 +285,12 @@ ctrlr_worker(void *arg)
> > cpu_set_t cpuset;
> > pthread_t thread;
> >
> > + if (ctrlr == NULL || ctrlr->bdev == NULL) {
> > + fprintf(stdout, "%s: Error, invalid argument passed to worker
> > thread\n",
>
> Might be better to print to stderr.
>
Yes, this looks a typo on my part.
> > + __func__);
> > + return NULL;
>
> Might be better to exit() directly like what the other
> error handling in this thread does:
>
> https://github.com/DPDK/dpdk/blob/bdcfcceb7a0b7534a0dba669279d18bd0f98d5e5/examples/vhost_scsi/vhost_scsi.c#L296-L299
>
> Otherwise destroy_device() will hang on sem_wait(&exit_sem):
>
> https://github.com/DPDK/dpdk/blob/bdcfcceb7a0b7534a0dba669279d18bd0f98d5e5/examples/vhost_scsi/vhost_scsi.c#L390
>
Ok, will change in next revision
