[cairo] [cairo-commit] 2 commits - src/cairo-surface.c
chris at chris-wilson.co.uk
Thu Dec 30 02:39:15 PST 2010
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
> 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 Wilson, Intel Open Source Technology Centre
More information about the cairo