Basic memory pool, alignment, thread safety

Question

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);
}

Answer 1

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;
}