[cairo] Review of: [cairo-mutex] Warn if no mutex definition found and let sanity macros err about undefined CAIRO_MUTEX macros.

[Behdad Esfahbod]

On Mon, 2007-04-23 at 15:55 -0400, Carl Worth wrote:
> >     [cairo-mutex] Warn if no mutex definition found and let sanity macros err
> >     about undefined CAIRO_MUTEX macros.
> >
> > diff --git a/src/cairo-mutex-private.h b/src/cairo-mutex-private.h
> > index 5150b8e..ad0a941 100644
> > --- a/src/cairo-mutex-private.h
> > +++ b/src/cairo-mutex-private.h
> > @@ -134,10 +134,12 @@ typedef int cairo_mutex_t;
> >
> >  #else /**********************************************************************/
> >
> > -# define CAIRO_MUTEX_LOCK(name)
> > -# define CAIRO_MUTEX_UNLOCK(name)
> > +# warning "XXX: No mutex implementation found.  Define CAIRO_NO_MUTEX to 1" \
> > +          "     to compile cairo without thread-safety support."
> > +
> > +#endif
> 
> I don't like this.
> 
> What we had in 1.4.2 and earlier was the following:
> 
> 	# error "No mutex declarations. Cairo will not work with multiple threads." \
>         "(Remove this #error directive to acknowledge & accept this limitation)."
> 
> and I was pretty happy with that. If you _really_ needed a cairo build
> without mutexes for some reason, and you were willing to limit the use
> of cairo to only single-threaded applications, then you could
> physically modify the cairo source code and remove an error that
> explicitly said "Cairo will not work with multiple threads".
> 
> I like how explicit the acknowledgement was in that case.
> 
> Now, apparently we broke that in 1.4.4, (I don't know exactly how
> things ended up if you tried to compile that without mutexes). But at
> least that error directive was gone.
> 
> I think adding a CAIRO_NO_MUTEX define is too weak. If we are going to
> make this work with a define rather than modifying the source, then
> I'd at least like to give it a much scarier name:
> 
> 	CAIRO_WILL_NOT_WORK_WITH_MULTIPLE_THREADS
> 
> or something like that. Or we can just put the #error back in.
> 
> But this is a sort of "build API" change, so we probably should have
> talked about it on the list before committing it.

Yeah, I noticed that the error was gone in 1.4.4, and tried to fix it.
And in fact, the whole idea of introducing CAIRO_NO_MUTEX was motivated
by your comment about single-threaded cairo usage:


(03:06:58 PM) Behdad Esfahbod: Behdad Esfahbod: cworth: so we want
compilation to abbort if no mutex impl found?
(03:08:13 PM) cworth: behdad: I don't know. Some people seem quite bent
on doing "single-threaded only" cairo usage.
(03:08:13 PM) eugen [n=eugen at cd1.cd.TU-Cottbus.De] entered the room.
(03:08:26 PM) Behdad Esfahbod: ok
(03:08:27 PM) cworth: behdad: Maybe require a --disable-<something> to
get that though...
(03:08:32 PM) Behdad Esfahbod: ok
(03:08:43 PM) cworth: (Though, then again, these same people might not
be using configure...)
(03:08:52 PM) ***cworth shrugs
(03:08:58 PM) Behdad Esfahbod: I handle it


And CAIRO_NO_MUTEX is the part that we can later AC_DEFINE in
configure.in.  So, if you prefer a longer uglier name, fine with me.  If
you want to remove it completely, again fine with me.


> -Carl
-- 
behdad
http://behdad.org/

"Those who would give up Essential Liberty to purchase a little
 Temporary Safety, deserve neither Liberty nor Safety."
        -- Benjamin Franklin, 1759