On Thu, Dec 12, 2013 at 01:59:00AM +0000, Upinder Malhi (umalhi) wrote:
> Hi Dan,
> Can I ask what static checker you are using for these?
>
In this email, the released version of Smatch will only warn about the
actual bug and not the others. It requires that you build the cross
function database though before you test the code.
~/smatch/smatch_scripts/build_kernel_data.sh
> > 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.
It doesn't cause a bug, but it's unusual and considered sloppy. APIs
should be clear about what error codes they return and they should
almost always return either ERR_PTRs *or* NULL but not *both*. The
exceptions are when NULL is a valid return which means something. For
example, maybe you are looking for a file and you return NULL if the
file is not found but ERR_PTR(-ENOMEM) if you have an error.
In linux if we decide that iommu_domain_alloc() should return ERR_PTRs
instead of NULL pointers, then it's up to the person who decides that
to change all the callers as well.
Also if you trust the lower levels to print their error messages
correctly then the error handling here could be much simpler. kzalloc()
for example, has very extensive error messages.
pd->domain = domain = iommu_domain_alloc(&pci_bus_type);
if (!domain) {
kfree(pd);
return -ENOMEM;
}
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
