Memory allocation in C

Question

the following is a very very simple version of malloc() and seems to allocate some space to me, but apart from the fact that there is no free() and I don't check if I've overrun the allocated space, how can I check that the code is correct?

Any obvious blunders that "C" experts would slap me for?

#include <stdio.h>
#include <unistd.h>

#define MAX_MEMORY 1024 * 1024 * 2 /* 2MB of memory */

void *stack = NULL; /* pointer to available stack */
void * memoryAlloc(size) {
    if (stack == NULL)
        stack = sbrk(MAX_MEMORY); /* give us system memory */

    void *pointer;
    pointer = (void *)stack + size; /* we always have space :) */
    stack += size; /* move in stack forward as space allocated */
    return pointer;
}

Answer 1

In addition to the basic problems Ned Batchelder pointed out, a much more subtle problem is that an allocator has to return an address that's properly aligned for whatever object is being allocated. On some platforms (x86) this may not matter except for performance issues, but on many platforms it's a complete deal breaker.

I also had to perform a (char*) cast to perform the stack pointer arithmetic (you can't do pointer arithmetic on void* types).

And you should put parens around the expression in the MAX_MEMORY macro. I don't think there are any precedence problems you'd get into without them, as all the high precedence operators than multiplication wouldn't be correct syntax anyway. With macros, it's always better safe than sorry. (There's at least one exception where the [] operator could bind only to the 2 and not the whole MAX_MEMORY expression, but it would be a very weird situation to see MAX_MEMORY[arrayname], even if it's syntactically valid).

As a matter of fact, I would have made it an enum.

You can probably keep the allocator simple by returning a block of memory that's properly aligned for any basic data type on the system (maybe an 8 byte alignment):

/* Note: the following is untested                   */
/*       it includes changes suggested by Batchelder */

#include <stdio.h>
#include <unistd.h>

enum {
    kMaxMemory = 1024 * 1024 * 2, /* 2MB of memory */
    kAlignment = 8
};

void *stack = NULL; /* pointer to available stack */
void * memoryAlloc( size_t size) {
    void *pointer;

    size = (size + kAlignment - 1) & ~(kAlignment - 1);   /* round size up so allocations stay aligned */

    if (stack == NULL)
    stack = sbrk(kMaxMemory); /* give us system memory */

    pointer = stack; /* we always have space :) */
    stack = (char*) stack + size;   /* move in stack forward as space allocated */
    return pointer;
}