Re: [PATCH v2] drm/panfrost: Document base field location constraint in panfrost_gem_object

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Rob Herring <robh+dt@kernel.org>,
	Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>,
	dri-devel@lists.freedesktop.org,
	Steven Price <steven.price@arm.com>
Subject: Re: [PATCH v2] drm/panfrost: Document base field location constraint in panfrost_gem_object
Date: Fri, 29 Nov 2019 20:24:37 +0100	[thread overview]
Message-ID: <20191129202437.0a3c6c25@collabora.com> (raw)
In-Reply-To: <20191129184038.GK624164@phenom.ffwll.local>

On Fri, 29 Nov 2019 19:40:38 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, Nov 29, 2019 at 03:19:36PM +0100, Boris Brezillon wrote:
> > On Fri, 29 Nov 2019 14:13:33 +0000
> > Steven Price <steven.price@arm.com> wrote:
> >   
> > > On 29/11/2019 13:56, Boris Brezillon wrote:  
> > > > I've spent hours chasing a memory corruption that was caused by
> > > > insertion of an extra field field before ->base. Let's document the
> > > > fact that base has to be the first field in panfrost_gem_object.
> > > > 
> > > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>    
> > > 
> > > This seems to be a limitation imposed by the gem_create_object()
> > > callback - e.g. it's assumed that kfree() can be directly called on the
> > > result. Useful to have the documentation though.  
> > 
> > Oh, you're right, I didn't catch that one.  
> 
> As a general rule of thumb, never insert anything before a struct member
> called base. Even more so if it's of the same kind of $thing, but less
> spezialed. This pattern is so common it's fairly often not documented
> anywhere.

I could argue that anything using container_of() in its to_xxx() helper
is misleading the user into thinking the position of the base field
doesn't matter (which is exactly what happened here), but I feel like I
already lost this battle, so I'll simply drop the patch.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel