[cairo] [cairo-commit] 2 commits - src/cairo-surface.c

[Chris Wilson]

On Thu, 30 Dec 2010 10:58:31 +0100, Andrea Canciani <ranma42 at gmail.com> wrote:
> On Thu, Dec 30, 2010 at 10:18 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > Minor comment on another wise good patch.
> >
> > On Wed, 29 Dec 2010 10:07:30 -0800 (PST), ranma42 at kemper.freedesktop.org (Andrea Canciani) wrote:
> >> commit 59ac884c607c024d0608cf7dec52509d9e9e328e
> >> Author: Uli Schlachter <psychon at znc.in>
> >> Date: Â  Sat Dec 25 23:39:21 2010 +0100
> >>
> >> Â  Â  Verify that surfaces leak no snapshots
> >>
> >> Â  Â  Finished surfaces should own no snapshots, because finished surfaces
> >> Â  Â  can't be used as sources, thus their snapshots would never be used.
> >>
> >> Â  Â  When free'ing the surface in cairo_surface_destroy(), it should have
> >> Â  Â  no snapshots, or they will be leaked.
> >>
> >> Â  Â  Signed-off-by: Uli Schlachter <psychon at znc.in>
> >>
> >> diff --git a/src/cairo-surface.c b/src/cairo-surface.c
> >> index 01ea27a..bc80d08 100644
> >> --- a/src/cairo-surface.c
> >> +++ b/src/cairo-surface.c
> >> @@ -654,6 +654,9 @@ cairo_surface_destroy (cairo_surface_t *surface)
> >> Â  Â  Â if (surface->owns_device)
> >> Â  Â  Â  Â  Â cairo_device_destroy (surface->device);
> >>
> >> + Â  Â assert (surface->snapshot_of == NULL);
> >> + Â  Â assert (!_cairo_surface_has_snapshots (surface));
> >
> > We like a space between the logical negation and its object:
> >
> > Â assert (! _cairo_surface_has_snapshots (surface));
> >
> > It just helps to make that operator more clear in dense code.
> 
> Oops, I didn't know about it.
> Should we add this to CODING_STYLE?

Naughty Carl, he told me off for something not in his CODING_STYLE and now
I've done the same. :)

> It would be a good idea to point out if "!" should always be followed
> by a space or which exceptions we want (like "!!").
> Some "missing" whitespaces are pointed out by:
>   git grep '![^ =]' src/

Right. !bad, !! good. Also inside a cpp block we can't have the space and
most of the 'if (!bad)' should be 'if (bad == NULL)', if anyone feels
janitorial.
 
> Andrea
> 
> PS: I'd love an automated way to point out where we're not respecting
> CODING_STYLE. I know that there are sometimes legitimate reasons
> to ignore it, but it would be nice to be notified about it, so that we only
> do it intentionally.

Indeed, something we can tie into make CC='coding-style gcc' to have
inline warnings would be ideal. Then tie that into a 'make checkpatch'.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre