bug-gnulib
[Top][All Lists]
Advanced

[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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]