[Python-Dev] GC head alignment issue

Paul Svensson paul@svensson.org
Fri, 12 Oct 2001 08:10:33 -0400 (EDT)


On Thu, 11 Oct 2001, Tim Peters wrote:

>[Paul Svensson]
>> The usual thing to do in this situation would be:
>>
>> typedef union _gc_head {
>> 	struct {
>> 		union _gc_head *internal_gc_next;
>> 		union _gc_head *internal_gc_prev;
>> 		int internal_gc_refs;
>> 	} gc_internals;
>> 	double dummy;  /* force worst-case alignment */
>> } PyGC_Head;
>>
(fixed some typos)
>> #define gc_next	gc_internals.internal_gc_next
>> #define gc_prev	gc_internals.internal_gc_prev
>> #define gc_refs	gc_internals.internal_gc_refs
>
>Yuck.  The less we abuse the preprocessor, the brighter the future of the
>Python codebase, and that's a good concrete example of why.

Tim,
Why do you consider this abuse ?  This pattern is used all over the place,
you can study it for example in the system headers for linux of glibc.
It's perfectly legitimate to not want to spread knowledge of the innards
of a struct (it should be wrapped another layer, yes) all over the place.
Now, if you claim that for this particular struct,
hiding the internals is irrelevant, that's an argument I can buy.

I have anuther issue with the change tho: as written, it forces
the struct (it's only a union by accident) _gc_head to be aligned,
but is it obvious that it really has to pad up the size too ?
If for example sizeof (int) is 4, sizeof (struct *) is 8 and
sizeof (double) is 16, with 8 byte alignment required for doubles,
would a compiler be allowed to make sizeof (struct _gc_head[1]) 24,
while still keeping sizeof (struct _gc_head) at 20 ?
This would make a difference when making it part of another struct.

Try this on for size:
--------------------------------------------------------------------------
typedef struct _gc_head {
	struct _gc_head *internal_gc_next;
	struct _gc_head *internal_gc_prev;
	union _gc_head_internals {
		int _gc_head_internal_gc_refs;
		double dummy;	  /* force worst-case alignment */
	}
} PyGC_Head;

#define gc_refs	_gc_head_internals._gc_head_internal_gc_refs
--------------------------------------------------------------------------

	/Paul