patterncMinor
Coroutines in C
Viewed 0 times
coroutinesstackoverflowprogramming
Problem
Please have a look at this little coroutines library
I'd appreciate a general code and style review, and your kind comments!
```
// 2>/dev/null; exec cc -c -Wall -Wextra -O2 ccoro.c ; exit $? # /
/*
* ccoro - Coroutines in C
* Sam Watkins, 2009
* this code is public domain
*
* ccoro uses setjmp and longjmp to achieve coroutines in plain C,
* non-preemptive threading. It works by allocating each thread some stack
* space on the normal stack. It makes sure that each thread has enough space
* to run using a padding variable, of size 8k by default, which is inserted by
* a function that starts the thread. Simple, huh? It uses some fairly simple
* trickery to create new threads that don't overlap with the other threads.
*
* I don't claim that this code is legal by the book, but it seems to work
* with at gcc, tcc, tendra and lcc on Linux i386 and x86_64, and with mingw
* gcc-4.4 and Visual C++ 98 Express on Windows.
*
* As is, this requires C99. To run with C89, use an enum for coro_pad.
*
* An earlier version was reported not to work with tendra on netbsd.
* I have fixed bugs since then so maybe it works now.
*
* An earlier version did not work with -O2 on some systems, it seems to be
* work
ccoro: http://sam.nipl.net/code/ccoroI'd appreciate a general code and style review, and your kind comments!
ccoro.h/*
* ccoro - Coroutines in C
* Sam Watkins, 2009
* this code is public domain
*
* ccoro uses setjmp and longjmp to achieve coroutines in plain C.
*/
#ifndef CCORO_H
#define CCORO_H 1
#ifdef __cplusplus
extern "C" {
#endif
#include
#if defined (__GNUC__)
#define noret void __attribute__((noreturn))
#else
#define noret void
#endif
extern int coro_pad;
enum { coro_code_done = -1, coro_code_alloc = -2, coro_code_dead = -3 };
typedef struct coro coro;
typedef void (*coro_func)(coro *caller);
struct coro
{
coro *next;
coro *prev;
jmp_buf j;
};
coro *new_coro(coro_func f);
int yield(coro **c);
int yield_val(coro **c, int val);
#ifdef __cplusplus
}
#endif
#endifccoro.c```
// 2>/dev/null; exec cc -c -Wall -Wextra -O2 ccoro.c ; exit $? # /
/*
* ccoro - Coroutines in C
* Sam Watkins, 2009
* this code is public domain
*
* ccoro uses setjmp and longjmp to achieve coroutines in plain C,
* non-preemptive threading. It works by allocating each thread some stack
* space on the normal stack. It makes sure that each thread has enough space
* to run using a padding variable, of size 8k by default, which is inserted by
* a function that starts the thread. Simple, huh? It uses some fairly simple
* trickery to create new threads that don't overlap with the other threads.
*
* I don't claim that this code is legal by the book, but it seems to work
* with at gcc, tcc, tendra and lcc on Linux i386 and x86_64, and with mingw
* gcc-4.4 and Visual C++ 98 Express on Windows.
*
* As is, this requires C99. To run with C89, use an enum for coro_pad.
*
* An earlier version was reported not to work with tendra on netbsd.
* I have fixed bugs since then so maybe it works now.
*
* An earlier version did not work with -O2 on some systems, it seems to be
* work
Solution
ccoro.h#define CCORO_H 1Random note, but you don't need to define that to
1, a simple #define CCORO_H would work fine.enum { coro_code_done = -1, coro_code_alloc = -2, coro_code_dead = -3 };I'd make it a little clearer that these are constants. Due to C's general lack of typing or other constructs to make code self-documenting, prefixing constants with
k or using ALL_CAPS is helpful.ccoro.cstatic noret new_coro_2(coro_func f, coro *caller);
static noret new_coro_3(coro_func f, coro *caller);
static int yield_val_2(coro *c, int val);I know you mentioned this already, but these function names are meaningless and confusing. Make them more meaningful to make the code clearer.
int coro_pad = 8192;What does this do? Add a comment to document this constant's purpose.
Also,
coro_pad is a constant, right? Why not use const to declare it as such?if (v == 0) /* yield */
longjmp(c->j, val);
else if (v == coro_code_alloc) { /* new - this is top */
coro *caller = current_coro;
current_coro = me;
new_coro_2(new_coro_f, caller);
/* cannot return */
}Braces, please! If you're going to have an
else clause with braces, put braces around the if clause, too.noret new_coro_3(coro_func f, coro *caller)
{ ... }Honestly, I have no idea what this function is doing. Changing the name would help, as mentioned above, but adding some inline comments would be nice, too!
Code Snippets
#define CCORO_H 1enum { coro_code_done = -1, coro_code_alloc = -2, coro_code_dead = -3 };static noret new_coro_2(coro_func f, coro *caller);
static noret new_coro_3(coro_func f, coro *caller);
static int yield_val_2(coro *c, int val);int coro_pad = 8192;if (v == 0) /* yield */
longjmp(c->j, val);
else if (v == coro_code_alloc) { /* new - this is top */
coro *caller = current_coro;
current_coro = me;
new_coro_2(new_coro_f, caller);
/* cannot return */
}Context
StackExchange Code Review Q#79415, answer score: 8
Revisions (0)
No revisions yet.