My own little memory manager in C

I have implemented a simple version of malloc() and its associated functions, free(), calloc(), and realloc().

• When free() is called, my implementation simply puts this memory on its own internal free-list.

• When malloc() is called, it first checks the internal free-list. Only if the memory is not available there, it calls sbrk() to request another chunk.

• All block sizes are based on powers of 2.

• A best fit strategy is used to allocate from the free-list.

Could you please review the same and let me know your suggestions/comments?

```
#include
#include
#include

// a struct representing a memory block
struct memoryBlock {
// metadata about memory block
size_t size;
struct memoryBlock *next;
};

// function declarations
void* malloc(size_t);
void free(void*);
void* calloc(size_t, size_t);
void realloc(void, size_t);
struct memoryBlock mymemcpy(void, const void*, size_t);
void fuse_adjacent_freeBlocks();
size_t round_to_next_power_of_two(unsigned int);
struct memoryBlock* get_bestFit_freeBlock(size_t size);
void print_freelist();

struct memoryBlock head = { 0, 0 };

/ This function allocates a block of memory of size bytes. /
void* malloc(size_t size) {

//If size is zero or less, return NULL
if (size size = size;

return ((char*) p) + sizeof(struct memoryBlock);
}

// traverse the free-list for a best-fit, if found return it
p = get_bestFit_freeBlock(size);
if (p != NULL) {
return ((char*) p) + sizeof(struct memoryBlock);
}

// reached only if best-fit not found, sbrk and return

p = (struct memoryBlock*) sbrk(size);
p->size = size;
return ((char*) p) + sizeof(struct memoryBlock);
}

/ This function frees a block of memory that had previously been allocated. /
void free(void *ptr) {

// If ptr is NULL, this function does nothing, just RETURNs
if (ptr == NULL) {
return;
}

struct memoryBlock to_free = (struct memoryBlock) (((char*) p

• You should separate your declarations into three parts:

• The public interface. malloc calloc realloc free

• The debug interface. print_freelist

• The implementation details, which should be kept out of headers. (the rest, consider static linkage)

• Avoid needless casts. Each and every cast is a possible trouble-spot.

• Use the standard library where allowed. No need to build your own square wheels.

• Comment on the why, not the what. The latter is already better expressed in code.

• Put the contract next to the declaration, not the definition.

Especially your malloc/realloc should mention they always fail a 0-sized request.

And realloc is an especially troublesome beast. (Consider reading the man-page or the c standard)

• There are many places you risk silent wrap-around. Be more careful! And pack the full blocksize-calculation into its own function.

static size_t roundUpToPowerOfTwo(size_t v) {
    // See https://graphics.stanford.edu/~seander/bithacks.html
    v--;
    v |= v >> 1;
    v |= v >> 2;
    v |= v >> 4;
    v |= v >> 8;
    v |= v >> 16;
    return v + 1;
}
static size_t getBlockSize(size_t v) {
    // reject too big requests
    if(v > (size_t)-1 / 2 + 1 - sizeof(struct memoryBlock))
        return (size_t)-1;
    v += sizeof(struct memoryBlock);
    return roundUpToPowerOfTwo(v);
}

• Your malloc contains needless duplication, and you fail to check sbrk succeeded. Consider this instead:

void* malloc(size_t size) {
    // Intentionally fail 0-size requests just because
    if (!size)
        return NULL;

    struct memoryBlock *p;

    size = getBlockSize(size);

    // Try to reuse old allocation
    if (head.next && (p = get_bestFit_freeBlock(size)))
        return p + 1;
    // Try to get new memory
    if((intptr_t)size size = size;
    return p + 1;
}

• calloc must check for overflow of the multiplication. Also, it must check for failure of malloc.

void* calloc(size_t nmemb, size_t size) {
    // Reject wrap-around and 0-requests
    size_t actualSize = nmemb * size;
    if(!size || actualSize / size != nmemb)
        return NULL;
    void *p = malloc(actualSize);
    if(p)
        memset(p, 0, actualSize);
    return p;
}

• Why don't you use fprintf(stderr, ...) in print_freelist?

• Consider rewriting your code so your blocks are always naturally aligned for better splitting and fusing.

StackExchange Code Review Q#155453, answer score: 8