Re: working C11 thread implementation

[Jens Gustedt <jens.gustedt@inria.fr>]

[-- Attachment #1: Type: text/plain, Size: 2868 bytes --]

Am Freitag, den 01.08.2014, 18:57 -0400 schrieb Rich Felker:
> On Sat, Aug 02, 2014 at 12:21:12AM +0200, Jens Gustedt wrote:
> > > > +int cnd_signal(cnd_t * cnd) {
> > > > +	/* In the best of all worlds this is a tail call. */
> > > > +	int ret = __pthread_cond_signal(cnd);
> > > > +	if (thrd_success)
> > > > +		return ret ? thrd_error : thrd_success;
> > > > +	return ret;
> > > > +}
> > > 
> > > this is a bit weird
> > > 
> > > i think it's better to just assume thrd_success==0
> > > and static assert it somewhere as a reminder
> > 
> > _Static_assert would need a compiler that implements this. It would be
> > easier just not to introduce a dependency for the tool chain. The code
> > as it is, now, will get optimized out by any decent compiler, I hope.
> 
> I'm fine with just assuming thrd_success is zero as long as we
> actually end up defining it as such.

I thought of this, but I am not in favor. History does not always turn
to the reasonable and obvious solution and there are many places where
the code would have to be updated if that assumption wouldn't hold. I
want these places clearly marked and grepable.

The day the C standard is amended to fix that constant to anyting (0
or non-0), I'll submit a patch that eliminates all of its uses. As
long as this is not specified in the standard, I'd like to stay with
it to ease maintenance.

> From a standpoint of minimizing the patch for adding C11 threads and
> making it C11-threads-only (rather than unrelated changes) I think the
> comment for pthread_self() should be omitted.

Generally, this is a matter of style. I often find that implementation
choices in musl could better be explained in place. For somebody new
to the sources (as I am) this often implies a lot of digging to find
the reason for a particular choice.

> But the comment is
> confusing as written anyway. Is your point that you're not making it
> an alias for __pthread_self because the latter is not necessarily a
> function?

yes, I'll try to formulate that better


> > If EINTR isn't 1, we have to be careful not to return -1 for some
> > other error code of SYS_nanosleep, whatever happens to be error number
> > 1. So the second case captures such an accidental -1 and sends -EINTR
> > (which we know not to be -1.).
> 
> This is kind of clever, but it might just be nicer to hard-code values
> like -1 and -2 rather than swapping -1/-EINTR so that the result looks
> like an errno value but really isn't one...

ok, I'll avoid that hackery

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]