[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] conditions variables module
From: |
Bruno Haible |
Subject: |
Re: [PATCH] conditions variables module |
Date: |
Fri, 8 Aug 2008 00:37:11 +0200 |
User-agent: |
KMail/1.5.4 |
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