On Tue, Jul 08, 2014 at 11:51:18AM +0200, Daniel Vetter wrote: > On Tue, Jul 08, 2014 at 07:47:13AM +0100, Chris Wilson wrote: > > On Mon, Jul 07, 2014 at 06:21:48PM -0700, Matt Roper wrote: > > > This should hopefully simplify the display code slightly and also > > > solves at least one mistake in intel_pipe_set_base() where > > > to_intel_framebuffer(fb)->obj is referenced during local variable > > > initialization, before 'if (!fb)' gets checked. > > > > > > Potential uses of this macro were identified via the following > > > Coccinelle patch: > > > > > > @@ > > > expression E; > > > @@ > > > * to_intel_framebuffer(E)->obj > > > > > > @@ > > > expression E; > > > identifier I; > > > @@ > > > I = to_intel_framebuffer(E); > > > ... > > > * I->obj > > > > > > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > > > > The only drawback is that I am suddenly nervous of potential NULL > > objs... > > I concur with Chris here, I think intel_fb_obj should be a static inline > and first check for fb == NULL. To catch abuse we could do an > > if (WARN_ON(!fb)) > return NULL; > > return to_intel_framebuffer(fb)->obj; > > The most likely abuse is that we call intel_fb_obj before checking fb for > NULL, so the WARN is better than a BUG_ON. With that I don't think we need > to change the checks as Chris suggested here. This came about because of one path where we should have expected fb to be NULL. Having a WARN followed by a GPF isn't any better than the GPF, in which case this patch is superfluous and you would rather just fix the single callsite where the bug occurred. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
