Patchwork [1/5] Add an explanation of when a function should be marked coroutine_fn

login
register
mail settings
Submitter Charlie Shepherd
Date Aug. 5, 2013, 6:44 p.m.
Message ID <1375728247-1306-2-git-send-email-charlie@ctshepherd.com>
Download mbox | patch
Permalink /patch/264747/
State New
Headers show

Comments

Charlie Shepherd - Aug. 5, 2013, 6:44 p.m.
From: Charlie Shepherd <cs648@cam.ac.uk>

Coroutine functions that can yield directly or indirectly should be annotated
with a coroutine_fn annotation. Add an explanation to that effect in
include/block/coroutine.h.
---
 include/block/coroutine.h | 3 +++
 1 file changed, 3 insertions(+)
Stefan Hajnoczi - Aug. 6, 2013, 8:39 a.m.
On Mon, Aug 05, 2013 at 08:44:03PM +0200, Charlie Shepherd wrote:
> From: Charlie Shepherd <cs648@cam.ac.uk>
> 
> Coroutine functions that can yield directly or indirectly should be annotated
> with a coroutine_fn annotation. Add an explanation to that effect in
> include/block/coroutine.h.
> ---
>  include/block/coroutine.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/block/coroutine.h b/include/block/coroutine.h
> index 377805a..3b94b6d 100644
> --- a/include/block/coroutine.h
> +++ b/include/block/coroutine.h
> @@ -37,6 +37,9 @@
>   * static checker support for catching such errors.  This annotation might make
>   * it possible and in the meantime it serves as documentation.
>   *
> + * A function must be marked with coroutine_fn if it can yield execution, either
> + * directly or indirectly.
> + *

This is correct except for the case of dynamic functions that do:

  if (qemu_in_coroutine()) {
  } else {
      Coroutine *co = qemu_coroutine_new(...);
      ...
  }

Here the function is coroutine_fn only if the caller is in coroutine
context.

I think your comment update should include a note about this.  When
you've split all dynamic functions into coroutine/non-coroutine versions
then the note can be removed.

Stefan
Charlie Shepherd - Aug. 8, 2013, 1:20 a.m.
On 06/08/2013 09:39, Stefan Hajnoczi wrote:
> On Mon, Aug 05, 2013 at 08:44:03PM +0200, Charlie Shepherd wrote:
>> From: Charlie Shepherd <cs648@cam.ac.uk>
>>
>> Coroutine functions that can yield directly or indirectly should be annotated
>> with a coroutine_fn annotation. Add an explanation to that effect in
>> include/block/coroutine.h.
>> ---
>>   include/block/coroutine.h | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/include/block/coroutine.h b/include/block/coroutine.h
>> index 377805a..3b94b6d 100644
>> --- a/include/block/coroutine.h
>> +++ b/include/block/coroutine.h
>> @@ -37,6 +37,9 @@
>>    * static checker support for catching such errors.  This annotation might make
>>    * it possible and in the meantime it serves as documentation.
>>    *
>> + * A function must be marked with coroutine_fn if it can yield execution, either
>> + * directly or indirectly.
>> + *
> This is correct except for the case of dynamic functions that do:
>
>    if (qemu_in_coroutine()) {
>    } else {
>        Coroutine *co = qemu_coroutine_new(...);
>        ...
>    }
>
> Here the function is coroutine_fn only if the caller is in coroutine
> context.
>
> I think your comment update should include a note about this.  When
> you've split all dynamic functions into coroutine/non-coroutine versions
> then the note can be removed.

Hmm ok, I can add a note to that effect.


Charlie

Patch

diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index 377805a..3b94b6d 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -37,6 +37,9 @@ 
  * static checker support for catching such errors.  This annotation might make
  * it possible and in the meantime it serves as documentation.
  *
+ * A function must be marked with coroutine_fn if it can yield execution, either
+ * directly or indirectly.
+ *
  * For example:
  *
  *   static void coroutine_fn foo(void) {