Re: [PATCH 05/29] drm/i915: prevent NULL pointer exception when using gmbus

[Chris Wilson <chris@chris-wilson.co.uk>]

On Sat, 14 Apr 2012 17:58:52 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Apr 13, 2012 at 09:26:14PM -0300, Eugeni Dodonov wrote:
> > On Fri, Apr 13, 2012 at 17:18, Chris Wilson <chris@chris-wilson.co.uk>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