diff options
author | Colin Cross <ccross@android.com> | 2013-12-20 19:43:50 -0500 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2014-02-07 17:21:09 -0500 |
commit | fee0c54e28f6ca187add93dfca226a8093cfa931 (patch) | |
tree | d62c8c58d90fda69cf035b36c457a8ce9df945ca /drivers/base | |
parent | 38dbfb59d1175ef458d006556061adeaa8751b72 (diff) |
dma-buf: avoid using IS_ERR_OR_NULL
dma_buf_map_attachment and dma_buf_vmap can return NULL or
ERR_PTR on a error. This encourages a common buggy pattern in
callers:
sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
if (IS_ERR_OR_NULL(sgt))
return PTR_ERR(sgt);
This causes the caller to return 0 on an error. IS_ERR_OR_NULL
is almost always a sign of poorly-defined error handling.
This patch converts dma_buf_map_attachment to always return
ERR_PTR, and fixes the callers that incorrectly handled NULL.
There are a few more callers that were not checking for NULL
at all, which would have dereferenced a NULL pointer later.
There are also a few more callers that correctly handled NULL
and ERR_PTR differently, I left those alone but they could also
be modified to delete the NULL check.
This patch also converts dma_buf_vmap to always return NULL.
All the callers to dma_buf_vmap only check for NULL, and would
have dereferenced an ERR_PTR and panic'd if one was ever
returned. This is not consistent with the rest of the dma buf
APIs, but matches the expectations of all of the callers.
Signed-off-by: Colin Cross <ccross@android.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/base')
-rw-r--r-- | drivers/base/dma-buf.c | 18 |
1 files changed, 11 insertions, 7 deletions
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index 1e16cbd61da2..cfe1d8bc7bb8 100644 --- a/drivers/base/dma-buf.c +++ b/drivers/base/dma-buf.c | |||
@@ -251,9 +251,8 @@ EXPORT_SYMBOL_GPL(dma_buf_put); | |||
251 | * @dmabuf: [in] buffer to attach device to. | 251 | * @dmabuf: [in] buffer to attach device to. |
252 | * @dev: [in] device to be attached. | 252 | * @dev: [in] device to be attached. |
253 | * | 253 | * |
254 | * Returns struct dma_buf_attachment * for this attachment; may return negative | 254 | * Returns struct dma_buf_attachment * for this attachment; returns ERR_PTR on |
255 | * error codes. | 255 | * error. |
256 | * | ||
257 | */ | 256 | */ |
258 | struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf, | 257 | struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf, |
259 | struct device *dev) | 258 | struct device *dev) |
@@ -319,9 +318,8 @@ EXPORT_SYMBOL_GPL(dma_buf_detach); | |||
319 | * @attach: [in] attachment whose scatterlist is to be returned | 318 | * @attach: [in] attachment whose scatterlist is to be returned |
320 | * @direction: [in] direction of DMA transfer | 319 | * @direction: [in] direction of DMA transfer |
321 | * | 320 | * |
322 | * Returns sg_table containing the scatterlist to be returned; may return NULL | 321 | * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR |
323 | * or ERR_PTR. | 322 | * on error. |
324 | * | ||
325 | */ | 323 | */ |
326 | struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, | 324 | struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, |
327 | enum dma_data_direction direction) | 325 | enum dma_data_direction direction) |
@@ -334,6 +332,8 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, | |||
334 | return ERR_PTR(-EINVAL); | 332 | return ERR_PTR(-EINVAL); |
335 | 333 | ||
336 | sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); | 334 | sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); |
335 | if (!sg_table) | ||
336 | sg_table = ERR_PTR(-ENOMEM); | ||
337 | 337 | ||
338 | return sg_table; | 338 | return sg_table; |
339 | } | 339 | } |
@@ -544,6 +544,8 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap); | |||
544 | * These calls are optional in drivers. The intended use for them | 544 | * These calls are optional in drivers. The intended use for them |
545 | * is for mapping objects linear in kernel space for high use objects. | 545 | * is for mapping objects linear in kernel space for high use objects. |
546 | * Please attempt to use kmap/kunmap before thinking about these interfaces. | 546 | * Please attempt to use kmap/kunmap before thinking about these interfaces. |
547 | * | ||
548 | * Returns NULL on error. | ||
547 | */ | 549 | */ |
548 | void *dma_buf_vmap(struct dma_buf *dmabuf) | 550 | void *dma_buf_vmap(struct dma_buf *dmabuf) |
549 | { | 551 | { |
@@ -566,7 +568,9 @@ void *dma_buf_vmap(struct dma_buf *dmabuf) | |||
566 | BUG_ON(dmabuf->vmap_ptr); | 568 | BUG_ON(dmabuf->vmap_ptr); |
567 | 569 | ||
568 | ptr = dmabuf->ops->vmap(dmabuf); | 570 | ptr = dmabuf->ops->vmap(dmabuf); |
569 | if (IS_ERR_OR_NULL(ptr)) | 571 | if (WARN_ON_ONCE(IS_ERR(ptr))) |
572 | ptr = NULL; | ||
573 | if (!ptr) | ||
570 | goto out_unlock; | 574 | goto out_unlock; |
571 | 575 | ||
572 | dmabuf->vmap_ptr = ptr; | 576 | dmabuf->vmap_ptr = ptr; |