Hi Slawomir, > -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Slawomir > Mrozowicz > Sent: Friday, February 03, 2017 9:18 AM > To: Doherty, Declan > Cc: dev@dpdk.org; Mrozowicz, SlawomirX > Subject: [dpdk-dev] [PATCH] cryptodev: fix segmentation fault > > This patch fix problem in function rte_cryptodev_devices_get(). > Program received signal SIGSEGV, Segmentation fault. > It also rework the function to use correct types and clean up visibility.
Are you fixing any Coverity issues with this patch? If so, state it here. > > Fixes: 38227c0e3ad2 ("cryptodev: retrieve device info") > > Signed-off-by: Slawomir Mrozowicz <slawomirx.mrozow...@intel.com> > --- > lib/librte_cryptodev/rte_cryptodev.c | 35 +++++++++++++++------------------- > - > lib/librte_cryptodev/rte_cryptodev.h | 4 ++-- > 2 files changed, 17 insertions(+), 22 deletions(-) > > diff --git a/lib/librte_cryptodev/rte_cryptodev.c > b/lib/librte_cryptodev/rte_cryptodev.c > index e557e77..10a59ba 100644 > --- a/lib/librte_cryptodev/rte_cryptodev.c > +++ b/lib/librte_cryptodev/rte_cryptodev.c > @@ -482,34 +482,29 @@ rte_cryptodev_count_devtype(enum > rte_cryptodev_type type) > return dev_count; > } > > -int > +uint8_t > rte_cryptodev_devices_get(const char *dev_name, uint8_t *devices, > - uint8_t nb_devices) > + const uint8_t nb_devices) I don't think it is necessary to use "const" here. Even if you change it in the function, the value wouldn't be changed outside. > { > - uint8_t i, cmp, count = 0; > - struct rte_cryptodev **devs = &rte_cryptodev_globals->devs; > - struct rte_device *dev; > - > - for (i = 0; i < rte_cryptodev_globals->max_devs && count < > nb_devices; > - i++) { > + uint8_t i, count = 0; > + struct rte_cryptodev *devs = rte_cryptodev_globals->devs; > + uint8_t max_devs = rte_cryptodev_globals->max_devs; > > - if ((*devs + i) > - && (*devs + i)->attached == > - RTE_CRYPTODEV_ATTACHED) > { > + for (i = 0; i < max_devs && count < nb_devices; i++) { > > - dev = (*devs + i)->device; > + if (devs[i].attached == RTE_CRYPTODEV_ATTACHED) { > + const struct rte_cryptodev_driver *drv = > devs[i].driver; > + int cmp = -1; Is it necessary to initialize this value to -1? cmp is going to be overwritten always. > > - if (dev) > - cmp = strncmp(dev->driver->name, > - dev_name, > - strlen(dev_name)); > + if (drv) > + cmp = strncmp(drv->pci_drv.driver.name, > + dev_name, > strlen(dev_name)); > else > - cmp = strncmp((*devs + i)->data->name, > - dev_name, > - strlen(dev_name)); > + cmp = strncmp(devs[i].data->name, > + dev_name, > strlen(dev_name)); > > if (cmp == 0) > - devices[count++] = (*devs + i)->data- > >dev_id; > + devices[count++] = devs[i].data->dev_id; > } > } >