[PATCH 01/13] drm/mgag200: Remove unnecessary NULL check in bo_unref
On Mon, Apr 7, 2014 at 5:19 PM, Ian Romanick wrote: > On 04/05/2014 02:44 AM, Daniel Vetter wrote: >> ttm_bo_unref unconditionally calls kref_put on it's argument, so the >> thing can't be NULL without already causing Oopses. > > Doesn't this mean the NULL check is in the wrong place (rather than the > NULL check should be removed)? Afaics chasing callchains it's a bug to call this with NULL pointer and no one really should be doing it. Like David Herrmann said it's sometimes useful if unref/free functions automatically cope with NULL, but ttm buffers don't seem to be of this kind. So consistency with other ttm drivers seems better, same with all the gem_free_object callbacks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 01/13] drm/mgag200: Remove unnecessary NULL check in bo_unref
On 04/07/2014 12:56 PM, Daniel Vetter wrote: > On Mon, Apr 7, 2014 at 5:19 PM, Ian Romanick wrote: >> On 04/05/2014 02:44 AM, Daniel Vetter wrote: >>> ttm_bo_unref unconditionally calls kref_put on it's argument, so the >>> thing can't be NULL without already causing Oopses. >> >> Doesn't this mean the NULL check is in the wrong place (rather than the >> NULL check should be removed)? > > Afaics chasing callchains it's a bug to call this with NULL pointer > and no one really should be doing it. Like David Herrmann said it's > sometimes useful if unref/free functions automatically cope with NULL, > but ttm buffers don't seem to be of this kind. So consistency with > other ttm drivers seems better, same with all the gem_free_object > callbacks. That's fair. I'm convinced. Patches 1, 3, and 5 are also Reviewed-by: Ian Romanick > -Daniel
[PATCH 01/13] drm/mgag200: Remove unnecessary NULL check in bo_unref
On 04/05/2014 02:44 AM, Daniel Vetter wrote: > ttm_bo_unref unconditionally calls kref_put on it's argument, so the > thing can't be NULL without already causing Oopses. Doesn't this mean the NULL check is in the wrong place (rather than the NULL check should be removed)? > Spotted by coverity. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/mgag200/mgag200_main.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c > b/drivers/gpu/drm/mgag200/mgag200_main.c > index 26868e5c55b0..0722d18992f4 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_main.c > +++ b/drivers/gpu/drm/mgag200/mgag200_main.c > @@ -322,9 +322,7 @@ static void mgag200_bo_unref(struct mgag200_bo **bo) > > tbo = &((*bo)->bo); > ttm_bo_unref(); > - if (tbo == NULL) > - *bo = NULL; > - > + *bo = NULL; > } > > void mgag200_gem_free_object(struct drm_gem_object *obj) >
[PATCH 01/13] drm/mgag200: Remove unnecessary NULL check in bo_unref
ttm_bo_unref unconditionally calls kref_put on it's argument, so the thing can't be NULL without already causing Oopses. Spotted by coverity. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/mgag200/mgag200_main.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index 26868e5c55b0..0722d18992f4 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -322,9 +322,7 @@ static void mgag200_bo_unref(struct mgag200_bo **bo) tbo = &((*bo)->bo); ttm_bo_unref(); - if (tbo == NULL) - *bo = NULL; - + *bo = NULL; } void mgag200_gem_free_object(struct drm_gem_object *obj) -- 1.8.5.2
