From: Ian Romanick <idr@freedesktop.org>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Jones <davej@redhat.com>,
DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 01/13] drm/mgag200: Remove unnecessary NULL check in bo_unref
Date: Mon, 07 Apr 2014 13:08:43 -0700 [thread overview]
Message-ID: <534305CB.3040500@freedesktop.org> (raw)
In-Reply-To: <CAKMK7uE6z0s494+u9F3gP2muNeCWnMk+YFuvo=uzge7AxD_ymA@mail.gmail.com>
On 04/07/2014 12:56 PM, Daniel Vetter wrote:
> On Mon, Apr 7, 2014 at 5:19 PM, Ian Romanick <idr@freedesktop.org> 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 <ian.d.romanick@intel.com>
> -Daniel
next prev parent reply other threads:[~2014-04-07 20:08 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-05 9:44 [PATCH 00/13] coverity Daniel Vetter
2014-04-05 9:44 ` Daniel Vetter
2014-04-07 16:01 ` David Herrmann
2014-04-05 9:44 ` [PATCH 01/13] drm/mgag200: Remove unnecessary NULL check in bo_unref Daniel Vetter
2014-04-07 15:19 ` Ian Romanick
2014-04-07 19:56 ` Daniel Vetter
2014-04-07 20:08 ` Ian Romanick [this message]
2014-04-05 9:44 ` [PATCH 02/13] drm/mgag200: Remove unecessary NULL check in gem_free Daniel Vetter
2014-04-07 15:23 ` Ian Romanick
2014-04-05 9:44 ` [PATCH 03/13] drm/cirrus: Remove unnecessary NULL check in bo_unref Daniel Vetter
2014-04-05 9:44 ` [PATCH 04/13] drm/cirrus: Remove unecessary NULL check in gem_free Daniel Vetter
2014-04-07 15:25 ` Ian Romanick
2014-04-05 9:44 ` [PATCH 05/13] drm/ast: Remove unnecessary NULL check in bo_unref Daniel Vetter
2014-04-05 9:44 ` [PATCH 06/13] drm/mgag200: Remove unecessary NULL check in gem_free Daniel Vetter
2014-04-05 16:17 ` [PATCH] drm/ast: " Daniel Vetter
2014-04-07 15:26 ` [PATCH 06/13] drm/mgag200: " Ian Romanick
2014-04-05 9:44 ` [PATCH 07/13] drm/via: Remove unecessary NULL check Daniel Vetter
2014-04-07 15:51 ` David Herrmann
2014-04-07 17:08 ` Daniel Vetter
2014-04-05 9:44 ` [PATCH 08/13] drm/ast: Remove dead code from cbr_scan2 Daniel Vetter
2014-04-07 15:28 ` Ian Romanick
2014-04-07 19:54 ` Daniel Vetter
2014-04-05 9:44 ` [PATCH 09/13] drm/udl: Initialize ret in udl_driver_load Daniel Vetter
2014-04-05 9:44 ` [PATCH 10/13] drm/bochs: Remove unnecessary NULL check in bo_unref Daniel Vetter
2014-04-05 9:45 ` [PATCH 11/13] drm/bochs: Remove unecessary NULL check in gem_free Daniel Vetter
2014-04-07 15:28 ` Ian Romanick
2014-04-05 9:45 ` [PATCH 12/13] drm/i2c/tda998x: Fix signed overflow issue Daniel Vetter
2014-04-07 15:31 ` Ian Romanick
2014-04-05 9:45 ` [PATCH 13/13] drm: Fix error handling in drm_master_create Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=534305CB.3040500@freedesktop.org \
--to=idr@freedesktop.org \
--cc=daniel.vetter@ffwll.ch \
--cc=davej@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox