Clarification of code comments and reality in malloc.h

[brett-walker]

Hi Devs,

I've been doing some deep code diving and I seem to have found a discrepancy between code comments and reality. This difference could hint at a bug or performance degradation, but I doubt it. I thought to ask to get clarification.

In malloc.h, the comments and definition for arena_header, ARENA_PADDING and free_arena_header are:

syslinux/core/mem/malloc.h

Lines 41 to 74 in 2ea44cb

	/*
	* This structure should be a power of two. This becomes the
	* alignment unit.
	*/
	struct arena_header {
	malloc_tag_t tag;
	size_t attrs; /* Bits 0..1: Type
	2..3: Heap,
	4..31: MSB of the size */
	struct free_arena_header *next, *prev;
	#ifdef DEBUG_MALLOC
	unsigned long _pad[3];
	unsigned int magic;
	#endif
	};
	/* Pad to 2*sizeof(struct arena_header) */
	#define ARENA_PADDING ((2 * sizeof(struct arena_header)) - \
	(sizeof(struct arena_header) + \
	sizeof(struct free_arena_header *) + \
	sizeof(struct free_arena_header *)))
	/*
	* This structure should be no more than twice the size of the
	* previous structure.
	*/
	struct free_arena_header {
	struct arena_header a;
	struct free_arena_header *next_free, *prev_free;
	size_t _pad[ARENA_PADDING];
	};

In my environment, the size of int and pointer are both 32-bits (x86 32-bits). When compiled with DEBUG_MALLOC undefined; I get

sizeof(arena_header) == 16
sizeof(free_arena_header) == 56

Which disagrees with the comments. The comments do make a lot of sense to ensure good alignment. To make the code agree with the comments (the easier option); the definition of ARENA_PADDING needs to be altered to:

#define ARENA_PADDING (((2 * sizeof(struct arena_header)) - \
		       (sizeof(struct arena_header) + \
			sizeof(struct free_arena_header *) +	\
			sizeof(struct free_arena_header *))) / sizeof(size_t))

This includes a reduction in the size of the padding by the unit size of the padding, i.e sizeof(size_t). Given that this is the memory sub-system, this small change could have a significant impact.

What are your thoughts?

Feedback most welcome,
Brett