2
\$\begingroup\$

Mainly for practicing purposes, I'm trying to implement a simple, yet efficient memory pool. It's basically a linked list that is able to grow, with fixed sized members. Each node has a flag if it's free or not. I'm not sure if I'm doing it the right way, and especially confused about how to deal with alignment, and thread safety. I know the basic concept, just don't know how to implement in this case. Could you please help me how to improve my code? This is what I have now:

#include <stdbool.h> #include "mem_pool.h" typedef struct block Block; typedef struct block { Block *next; void *ptr; bool free; } Block; struct mem_pool { Block *head; void *memory; size_t memory_size; size_t memb_size; }; #define get_ptr_and_increase(pool, size) pool->memory; pool->memory += size #define add_block_size(pool) pool->memory_size += sizeof(Block) + pool->memb_size static Block *new_block(MemoryPool *pool, bool is_free) { Block *block = get_ptr_and_increase(pool, sizeof(Block)); block->ptr = get_ptr_and_increase(pool, pool->memb_size); block->next = NULL; block->free = is_free; return block; } MemoryPool *pool_init(size_t memb_size) { MemoryPool *pool = malloc(sizeof(MemoryPool)); pool->memb_size = memb_size; pool->memory_size = 0; add_block_size(pool); pool->memory = malloc(pool->memory_size); pool->head = new_block(pool, true); return pool; } static Block *find_free(MemoryPool *pool) { Block *current = pool->head; while (NULL != current && false == current->free) { current = current->next; } return current; } static void add_new_head(MemoryPool *pool) { Block *tmp = pool->head; pool->head = new_block(pool, false); pool->head->next = tmp; add_block_size(pool); pool->memory = realloc(pool->memory, pool->memory_size); } void *pool_alloc(MemoryPool *pool) { Block *free = find_free(pool); if (NULL != free) { return free->ptr; } add_new_head(pool); return pool->head->ptr; } void pool_free(MemoryPool *pool, void *block) { Block *current = pool->head; while (current->ptr != block) { current = current->next; } current->free = true; } void pool_destroy(MemoryPool *pool) { free(pool->memory - pool->memory_size); free(pool); }
\$\endgroup\$
2
  • \$\begingroup\$ Does this code compile? \$\endgroup\$ Commented Oct 23, 2016 at 20:50
  • \$\begingroup\$ It did last time. \$\endgroup\$ Commented Oct 23, 2016 at 21:53

1 Answer 1

Reset to default
4
\$\begingroup\$

Serious Bug

The moment you do this:

pool->memory = realloc(pool->memory, pool->memory_size);

all the existing head, next, ptr, and memory pointers become invalid, since they were pointing into the memory that was just freed. So essentially, your memory pool becomes useless after you expand it.

Unsafe macros

This macro:

#define get_ptr_and_increase(pool, size) pool->memory; pool->memory += size

is unsafe for multiple reasons:

  1. Arguments are not used inside parentheses, e.g. (pool)

  2. The two statements could get split unexpectedly, such as in:

    if (condition) p = get_ptr_and_increase(pool, size);

I suggest creating a static inline function instead:

static inline void *get_ptr_and_increase(MemoryPool *pool, size_t size) { void *ret = pool->memory; pool->memory += size; ret += size; }
\$\endgroup\$
2
  • \$\begingroup\$ Thanks. What kind of techniques are there to implement a dynamically growing pool? \$\endgroup\$ Commented Oct 24, 2016 at 16:28
  • 2
    \$\begingroup\$ @Isty001 First, I would allocate large chunks instead of one element at a time. Second, I would never free or realloc anything. I would instead allocate a new large chunk when the first chunk got completely used up. \$\endgroup\$ Commented Oct 24, 2016 at 17:21

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.