Re: [Qemu-devel] [PATCH 2/3] coroutine: Clean up qemu_coroutine_enter()

[Stefan Hajnoczi <stefanha@gmail.com>]

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

On Tue, Feb 10, 2015 at 11:41:27AM +0100, Kevin Wolf wrote:
> qemu_coroutine_enter() is now the only user of coroutine_swap(). Both
> functions are short, so inline it.
> 
> Also, using COROUTINE_YIELD is now even more confusing because this code
> is never called during qemu_coroutine_yield() any more. In fact, this
> value is never read back, so we can just introduce a new COROUTINE_ENTER
> which documents the purpose of the task switch better.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  include/block/coroutine_int.h |  1 +
>  qemu-coroutine.c              | 36 +++++++++++++++---------------------
>  2 files changed, 16 insertions(+), 21 deletions(-)
> 
> diff --git a/include/block/coroutine_int.h b/include/block/coroutine_int.h
> index f133d65..69b83db 100644
> --- a/include/block/coroutine_int.h
> +++ b/include/block/coroutine_int.h
> @@ -29,6 +29,7 @@
>  #include "block/coroutine.h"
>  
>  typedef enum {
> +    COROUTINE_ENTER = 0,

This makes the ucontext code harder to understand because
CoroutineAction values are used with setjmp()/longjmp() in
qemu_coroutine_switch().

The longjmp() man page says:

  If longjmp() is invoked with a second argument of 0, 1 will be
  returned instead.

I haven't checked whether or not this causes problems, but the code
would be simpler if we avoided using 0.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]