diff mbox

[2/3] qemu-thread: add cleanup_push() and cleanup_pop()

Message ID 1275118686-15649-3-git-send-email-corentincj@iksaif.net
State New
Headers show

Commit Message

Corentin Chary May 29, 2010, 7:38 a.m. UTC
From pthread man:

 These  functions  manipulate  the  calling thread's stack of
 thread-cancellation clean-up handlers.  A clean-up handler is
 a function that is automatically executed when a thread is canceled
 [...] it might, for example, unlock a mutex so that it becomes
 available to other threads in the process.

These two functions are implemented using macros because there is no
other way to do that (pthread man, again):

 On Linux, the pthread_cleanup_push() and pthread_cleanup_pop()
 functions are implemented as macros that expand to text  containing
 '{'  and  '}',  respectively.  This means that variables declared
 within the scope of paired calls to these functions will only
 be visible within that scope.

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 qemu-thread.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

Comments

Paul Brook June 3, 2010, 5:50 a.m. UTC | #1
> From pthread man:
> 
>  These  functions  manipulate  the  calling thread's stack of
>  thread-cancellation clean-up handlers.  A clean-up handler is
>  a function that is automatically executed when a thread is canceled
>  [...] it might, for example, unlock a mutex so that it becomes
>  available to other threads in the process.

Do we really need to use thread cancellation?
It's one of those features that makes me extremely nervous. Especially in C 
code where people generally aren't expecting exceptions to be thrown.

Paul
Paolo Bonzini June 3, 2010, 7:27 a.m. UTC | #2
On 05/29/2010 09:38 AM, Corentin Chary wrote:
> Signed-off-by: Corentin Chary<corentincj@iksaif.net>
> ---
>   qemu-thread.h |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-thread.h b/qemu-thread.h
> index 19bb30c..e5006bb 100644
> --- a/qemu-thread.h
> +++ b/qemu-thread.h
> @@ -41,4 +41,8 @@ void qemu_thread_self(QemuThread *thread);
>   int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
>   void qemu_thread_exit(void *retval);
>
> +#define qemu_thread_cleanup_pop(execute) pthread_cleanup_pop(execute)
> +#define qemu_thread_cleanup_push(routine, arg)  \
> +    pthread_cleanup_push(routine, arg)

I agree with Paul that this isn't necessary.  Also you're not using 
pthread_exit.  Probably stale from a previous version of the patch?

Paolo
Corentin Chary June 3, 2010, 7:46 a.m. UTC | #3
On Thu, Jun 3, 2010 at 9:27 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 05/29/2010 09:38 AM, Corentin Chary wrote:
>>
>> Signed-off-by: Corentin Chary<corentincj@iksaif.net>
>> ---
>>  qemu-thread.h |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/qemu-thread.h b/qemu-thread.h
>> index 19bb30c..e5006bb 100644
>> --- a/qemu-thread.h
>> +++ b/qemu-thread.h
>> @@ -41,4 +41,8 @@ void qemu_thread_self(QemuThread *thread);
>>  int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
>>  void qemu_thread_exit(void *retval);
>>
>> +#define qemu_thread_cleanup_pop(execute) pthread_cleanup_pop(execute)
>> +#define qemu_thread_cleanup_push(routine, arg)  \
>> +    pthread_cleanup_push(routine, arg)
>
> I agree with Paul that this isn't necessary.  Also you're not using
> pthread_exit.  Probably stale from a previous version of the patch?

Right pthread_exit() is missing.

Anyway, I don't use thread cancellation in the threaded vnc server (I
send a signal to the condition with an empty queue to stop the
thread), so we can skip this patch.
diff mbox

Patch

diff --git a/qemu-thread.h b/qemu-thread.h
index 19bb30c..e5006bb 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -41,4 +41,8 @@  void qemu_thread_self(QemuThread *thread);
 int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
 void qemu_thread_exit(void *retval);
 
+#define qemu_thread_cleanup_pop(execute) pthread_cleanup_pop(execute)
+#define qemu_thread_cleanup_push(routine, arg)  \
+    pthread_cleanup_push(routine, arg)
+
 #endif