[PATCH weston 4/5] window: merge struct surface_data into shm_surface_data

[Pekka Paalanen]

On Mon, 19 Nov 2012 16:05:58 -0500
Kristian Høgsberg <hoegsberg at gmail.com> wrote:

> On Mon, Nov 19, 2012 at 03:32:50PM +0200, Pekka Paalanen wrote:
> > Struct surface_data was not really useful, and it definitely was not
> > used with EGL-based windows.
> > 
> > This also fixes a semantic mistake, where struct shm_surface_data was
> > put into cairo_surface_t private, but got out as struct surface_data
> > instead. Due to struct layout, however, this did not cause a real bug.
> 
> This wasn't a mistake - it's pretty common to put a struct into
> another struct as the first member and then just cast between the
> pointers.  Using container_of is more general and lets you put the
> embedded struct anywhere in the containing struct, but consistently
> placing the embedded struct first is a valid and common pattern.

My opposition in this particular case was that the cast was hidden.
There was no way to find it without carefully reviewing all
cairo_surface private set/get operations, tracking through which
cairo_surface object was used and what was the key.

I do tend to use container_of even when I don't have to, since IMO it
is more documentary. Whatever I put into a void* will be taken out
as the same type, then converted later, if easily possible. I prefer
this kind of consistency. Works also if you have more than one embedded
struct, like with lists.

Every time I find such a direct or a hidden cast, I have to go and check
the types are actually compatible.


Thanks,
pq