Hi Dan,
Can I ask what static checker you are using for these?
See Inline for rest.
On Dec 11, 2013, at 2:38 PM, Dan Carpenter <[email protected]> wrote:
> Hello Upinder Malhi,
>
> The patch b1819c455542: "IB/usnic: Add Cisco VIC low-level hardware
> driver" from Sep 10, 2013, leads to the following static checker
> warning:
> drivers/infiniband/hw/usnic/usnic_uiom.c:47 usnic_uiom_alloc_pd()
> warn: passing zero to 'PTR_ERR'"
>
> drivers/infiniband/hw/usnic/usnic_uiom.c
> 469 pd->domain = domain = iommu_domain_alloc(&pci_bus_type);
> ^^^^^^^^^^^^^^^^^^^
> This function returns NULL on error not error pointers.
>
> 470 if (IS_ERR_OR_NULL(domain)) {
> 471 usnic_err("Failed to allocate IOMMU domain with err
> %ld\n",
> 472 PTR_ERR(pd->domain));
> 473 kfree(pd);
> 474 return ERR_PTR(domain ? PTR_ERR(domain) : -ENOMEM);
> 475 }
>
> Similar harmless but crappy slop in:
[UM] - Unless I'm missing something, I think this is OK and preferred in case
iommu_domain_alloc in future starts returning ERR_PTRs.
>
> vers/infiniband/hw/usnic/usnic_ib_main.c
> 249 us_ibdev = (struct usnic_ib_dev
> *)ib_alloc_device(sizeof(*us_ibdev));
> 250 if (IS_ERR_OR_NULL(us_ibdev)) {
> ^^^^^^^^^^^^^^^^^^^^^^^^
> 251 usnic_err("Device %s context alloc failed\n",
> 252 netdev_name(pci_get_drvdata(dev)));
> 253 return ERR_PTR(us_ibdev ? PTR_ERR(us_ibdev) : -EFAULT);
> 254 }
> 255
> 256 us_ibdev->ufdev = usnic_fwd_dev_alloc(dev);
> 257 if (IS_ERR_OR_NULL(us_ibdev->ufdev)) {
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 258 usnic_err("Failed to alloc ufdev for %s with err
> %ld\n",
> 259 pci_name(dev),
> PTR_ERR(us_ibdev->ufdev));
> 260 goto err_dealloc;
> 261 }
>
> The general confusing about what return values are leads to a bug later
> on:
>
> vers/infiniband/hw/usnic/usnic_ib_main.c
> 462 pf = usnic_ib_discover_pf(vf->vnic);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This function returns ERR_PTRs. Also it has a bug and can return freed
> pointers. Oops... :(
[UM] - Yes, that is oops indeed. Will fix.
>
> 463 if (!pf) {
> 464 usnic_err("Failed to discover pf of vnic %s with
> err%d\n",
> 465 pci_name(pdev), err);
> 466 goto out_clean_vnic;
> 467 }
> 468
> 469 vf->pf = pf;
> 470 spin_lock_init(&vf->lock);
> 471 mutex_lock(&pf->usdev_lock);
>
> regards,
> dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html