[syslinux] Mismatch between code comments and reality in malloc.h
H. Peter Anvin
hpa at zytor.com
Mon Feb 5 09:58:36 PST 2018
On 02/03/18 04:28, Brett Walker via Syslinux wrote:
> 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 syslinux/core/mem/malloc.h, the comments and definition for arena_header, ARENA_PADDING and free_arena_header are:
>
>
> /*
> * 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 memory block 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.
>
Right, either ARENA_PADDING needs to be changed as you say, or _pad
needs to be defined as char (or uint8_t) instead of size_t. The latter
might be better.
-hpa
More information about the Syslinux
mailing list