On Tue, Jul 08, 2014 at 11:06:49AM +0100, Chris Wilson wrote: > 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. Ok, I've missed the context then. Anyway I like the idea of intel_fb_obj and that it'll make the code more robust against misorderings of fb == NULL checks. Please ignore me and proceed ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx