Hi Yoann,

> Attached is an updated patch:

Thanks.

> - Include a new glthread module (which doesn't handle sched_yield, since
> that might pull another dependency in. I guess we need yet another
> separate module for this one).

It's a pity, because one cannot write a reasonable test program that works
with GNU Pth without putting sched_yield calls here and there. But I guess
you're right, and it needs a 5th modules, because of the extra library
dependency.

> - Implement *basic* unit test for glthread_cond_wait() /
> glthread_cond_timedwait().

I would prefer if you could put this into a separate test-cond.c file -
so that test-lock.c does not need to depend on the 'cond' module.

gl_thread_sigmask ?! Sounds a bit advanced. I'm not sure this can be
implemented in a portable way, especially on Win32.

gl_thread_atfork ?! Sounds advanced and weird to implement as well.

I propose to clearly document that these two functions are not supported
on all platforms.

Now nitpicking:

  - The autoconf macros should invoke AC_C_INLINE, since you use 'inline'.

  - GNU coding style: indentation depth of 2, brace placement, space before
    function name and opening parenthesis, indentation as with
    "unexpand --first-only -t 8", etc.

  - Typo in glhread_cond_destroy

  - Macro glthread_cond_destroy(COND) should expand to an expression, and
    '(void)' is not an expression. Write 0 instead.

  - Insufficient parenthesizing of argument THREAD here (can be any expression
    of pointer type):
     # define glthread_create(THREAD, FUNC, ARG) \
         (pth_in_use () ? (((*THREAD) = pth_spawn (NULL, FUNC, ARG)) ? 0 : -1) 
: 0)

  - Naming of _gl_thread_create: In the lock module, I preferred to use a 
'_func'
    suffix: gl_thread_create_func.

Bruno



Reply via email to