From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 05/29] drm/i915: prevent NULL pointer exception when using gmbus Date: Sat, 14 Apr 2012 17:01:02 +0100 Message-ID: <1334419271_432905@CP5-2952> References: <1334347745-11743-1-git-send-email-eugeni.dodonov@intel.com> <1334347745-11743-6-git-send-email-eugeni.dodonov@intel.com> <1334348304_415235@CP5-2952> <20120414155852.GA4215@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from fireflyinternet.com (smtp.fireflyinternet.com [109.228.6.236]) by gabe.freedesktop.org (Postfix) with ESMTP id 891CB9E7BA for ; Sat, 14 Apr 2012 09:01:19 -0700 (PDT) In-Reply-To: <20120414155852.GA4215@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter , Eugeni Dodonov Cc: intel-gfx@lists.freedesktop.org, Eugeni Dodonov List-Id: intel-gfx@lists.freedesktop.org On Sat, 14 Apr 2012 17:58:52 +0200, Daniel Vetter wrote: > On Fri, Apr 13, 2012 at 09:26:14PM -0300, Eugeni Dodonov wrote: > > On Fri, Apr 13, 2012 at 17:18, Chris Wilson wrote: > > > > > On Fri, 13 Apr 2012 17:08:41 -0300, Eugeni Dodonov < > > > eugeni.dodonov@intel.com> wrote: > > > > Prevent a NULL pointer exception when we are trying to retrieve EDID data > > > > from non-existent adapter. > > > > > > This just means that a HDMI with a garbage ddc_bus is never detected. > > > Since we control ddc_bus entirely, it means that the initialisation is > > > completely bogus. Please fix the root cause and not paper over > > > programming bugs. > > > > > > > Sorry, I think I wasn't clear in my cover letter about this one.. I am not > > hitting any issue with this current branch I sent; but the nature of the > > code we have after the gmbus refactoring made me a bit paranoid about > > accessing properties of something that could potential be NULL. > > > > As far as I can see, we should not hit this with any combination of outputs > > on any hardware that is already covered by the wikipedia :). So I am fine > > with dropping this patch if you also think that it does not adds anything > > relevant to us. Or perhaps I should refactor it into adding some WARN's > > instead? > > If you're uneasy about a NULL deref add a BUG_ON (because the NULL-deref > will lead to an oops anyway) or add a WARN_ON and try to bail out. But if > the condition is truely (or at least should be) impossible, a BUG_ON to > document it is imo better (if we can't bail out in a sensible way). In this case, the paranoid would add the check during init (and fail to create the HDMI encoder with a loud DRM_ERROR). -Chris -- Chris Wilson, Intel Open Source Technology Centre