[cairo] Review of: [cairo-mutex] Add default implementation for CAIRO_MUTEX_INIT that uses CAIRO_MUTEX_NIL_INITIALIZER. This used to be the implementation for pthread because pthread_mutex_init() is broken. See d48bb4fbe876a93199ba48fcf5f32734fbe18ba9.

[Carl Worth]

> diff-tree 97c197478023ceb5477a203d058eaec2cb18f987 (from 6d2a2dd6d9190c62b209e47c083b7df72e7134fb)
> Author: Behdad Esfahbod <behdad at behdad.org>
> Date:   Thu Apr 19 16:26:21 2007 -0400
>
>     [cairo-mutex] Add default implementation for CAIRO_MUTEX_INIT
>     that uses CAIRO_MUTEX_NIL_INITIALIZER.  This used to be the
>     implementation for pthread because pthread_mutex_init() is
>     broken.  See d48bb4fbe876a93199ba48fcf5f32734fbe18ba9.
...
> +#ifndef CAIRO_MUTEX_INIT
> +# define CAIRO_MUTEX_INIT(_mutex) do {				\
> +    cairo_mutex_t _tmp_mutex = CAIRO_MUTEX_NIL_INITIALIZER;     \
> +    memcpy ((_mutex), &_tmp_mutex, sizeof (_tmp_mutex));        \
> +} while (0)
> +#endif

This is a bit confused. The original intent of
CAIRO_MUTEX_NIL_INITIALIZER is that it was something that would be
used to initialize the mutex of a nil object, (so it had to be able to
compile, but it didn't have to actually mean anything useful, since it
would never be looked at).

But if we're going to use it to initialize real mutex objects, then
that's a big semantic change, and its name is all wrong.

Meanwhile, I think this patch exacerbates a problem that started with
the big mutex rewrite and continued with addition of things like
CAIRO_MUTEX_USE_GENERIC_INITIALIZATION.

The problem is that one used to be able to examine the file defining
all the mutex stuff, find the block of interest for the relevant
platform, and directly see everything of interest there.

Now, however, it's a quite convoluted mess. After you find that block
you have to search the whole rest of the file, (which is already a bit
of an #ifdef nightmare), and realize that if CAIRO_MUTEX_INIT isn't
defined in the block of interest, then something will get
substituted. Then if CAIRO_MUTEX_INITIALIZE isn't defined then
something else will also get substituted.

I'd much rather see any common things defined first and the
substitutions happen explicitly within the small, platform-specific
blocks, (so that the reader doesn't have to mentally insert implicit
substitutions in order to understand what's happening).

Meanwhile, the names of CAIRO_MUTEX_INIT and CAIRO_MUTEX_INITIALIZE
are far too similar for having such distinct behaviors.

I'll attempt a fix of all of this shortly, (unless someone beats me to
it).

-Carl

PS. Behdad, your commit message didn't follow the "one-line summary"
convention.

PPS. If someone were to fix our cairo-commit email-sending hook to
send a separate message per commit, that would make review like this
much easier.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/cairo/attachments/20070423/9875f226/attachment.pgp