{ This is a partial reply; what with YAPC I didn't finish, but a new
version of the patch from Leo will give me time to catch up }
I've reviewed the patch. I appreciate what you're doing with it, but
there are some issues that will have to be addressed.
1. "union Parrot_Context" is not portable C.
The C standard requires that all structure pointers be bitwise
compatible - same size, layout, etc.[*] It doesn't require that of
other pointers -- specifically char* in this case. So the union:
typedef union Parrot_Context {
struct parrot_regs_t *bp; /* register base pointer */
struct Real_Context *rctx; /* realcontext is at rctx[-1] */
char *as_charp; /* to simplify allocation */
} parrot_context_t;
isn't guaranteed to work like you think it does.
There are two good approaches to what you're trying to do here that I
can recommend.
a. remove the as_charp member and do without it.
b. replace the union with a single pointer of whatever type is
convenient ... a structure or a void* or whatever ... and just
cast the pointer as needed. This isn't C++ ... pointer casts
are guaranteed to have zero runtime cost. It's just a macro.
I recommend (b).
2. Naming of "Real_Context".
"Real_Context" is not an OK name. I'm going to go into detail here
because there's a principle involved:
There's no reason ever to name something "real_foo" (except in the
mathematical real-number sense). Because that means something else is
a fake unreal foo, but it's not labelled such. If the "foo" becomes
fake, then it should be labelled so.
If there's a real foo and an unreal foo, then the real one should be
named "foo" and the unreal one should be named "fake_foo" or whatever.
3. REAL_CTX() must be renamed for the reasons given above. I think
CONTEXT() is a good name, but CTX() will do, if only because we could
very well going to have to change the name of "context" to something
more specific one of these days.
4. CTX_VAR(p,m) is a bad idea and should be simply removed. Usage of
CTX_VAR(p,m) should be replaced with CTX(p)->m. Remember code should
be greppable; it should be possible to find most (if not all) member
usage by grepping for ".member" and "->member".
5. FIRST_CTX_VAR is at least unnecessary, and might be a bug.
If you want to zero a structure, why go to the trouble of asking for
the address of its first member? Just use the address of the structure
as a whole. That's the real starting point for sizeof() anyway.
Even worse, I'm not sure the usage of FIRST_CTX_VAR is sound WRT the C
standard(s). Does anyone on the list know, given:
typedef struct {
whatever_t m;
} S;
... whether offsetof(S,m) is guaranteed to be zero? I think it's not,
and so the usage of FIRST_CTX_VAR for e.g. memset is not OK.
6. As long as we have the "parrot_context_t" typedef, we should use it
instead of "struct" or, now, "union Parrot_Context". No point in
hiding the difference between unions and structs under a nice typedef
wrapper if we're just going to rip off the wrapper ourselves.
{ and I stop here }
--
Chip Salzenberg <[EMAIL PROTECTED]>
