| Message ID | 1427211983-26284-3-git-send-email-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 24, 2015 at 03:46:20PM +0000, Michel Thierry wrote: > i915_dma_map_single relies on dma_mapping_error, which returns positive > error codes. Found by static checker. > > Introduced by commit 678d96fbb3b5995a2fdff2bca5e1ab4a40b7e968 > ("drm/i915: Track GEN6 page table usage"). > Please give me reporter credit so I can justify my paycheck to my boss. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Cc: Dan Carpenter <dan.carpenter@oracle.com> > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 645c363..79ade6f 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -348,7 +348,7 @@ static struct i915_page_table_entry *alloc_pt_single(struct drm_device *dev) > goto fail_page; > > ret = i915_dma_map_single(pt, dev); > - if (ret) > + if (ret != 0) Heh. What is this? :P You just added a double negative. Just change to return negative error codes on failure. regards, dan carpenter
On Tue, Mar 24, 2015 at 06:57:13PM +0300, Dan Carpenter wrote: > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index 645c363..79ade6f 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -348,7 +348,7 @@ static struct i915_page_table_entry *alloc_pt_single(struct drm_device *dev) > > goto fail_page; > > > > ret = i915_dma_map_single(pt, dev); > > - if (ret) > > + if (ret != 0) > > Heh. What is this? :P You just added a double negative. Just change > to return negative error codes on failure. Oops. I left out a word. That should say "Just change i915_dma_map_page_single() to negative error codes on failure." Currently, it returns 0 on success and 1 on failure which is non-standard. regards, dan carpenter
On 3/24/2015 4:03 PM, Dan Carpenter wrote: > On Tue, Mar 24, 2015 at 06:57:13PM +0300, Dan Carpenter wrote: >>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c >>> index 645c363..79ade6f 100644 >>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >>> @@ -348,7 +348,7 @@ static struct i915_page_table_entry *alloc_pt_single(struct drm_device *dev) >>> goto fail_page; >>> >>> ret = i915_dma_map_single(pt, dev); >>> - if (ret) >>> + if (ret != 0) >> Heh. What is this? :P You just added a double negative. Just change >> to return negative error codes on failure. > Oops. I left out a word. That should say "Just change > i915_dma_map_page_single() to negative error codes on failure." > > Currently, it returns 0 on success and 1 on failure which is > non-standard. Ok, the function is returning the value straight from dma_mapping_error(), I'll change i915_dma_map_page_single() to return -ENOMEM when dma_mapping_error doesn't return 0. Thanks, -Michel > regards, > dan carpenter >
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 645c363..79ade6f 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -348,7 +348,7 @@ static struct i915_page_table_entry *alloc_pt_single(struct drm_device *dev) goto fail_page; ret = i915_dma_map_single(pt, dev); - if (ret) + if (ret != 0) goto fail_dma; return pt;
i915_dma_map_single relies on dma_mapping_error, which returns positive error codes. Found by static checker. Introduced by commit 678d96fbb3b5995a2fdff2bca5e1ab4a40b7e968 ("drm/i915: Track GEN6 page table usage"). Cc: Dan Carpenter <dan.carpenter@oracle.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Signed-off-by: Michel Thierry <michel.thierry@intel.com> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)