10

I have implemented by own malloc and free using mmap. Now since unlike free, munmap also takes length as parameter, therefore I put length as an additional information in the mapped memory.

The code for my malloc and free is shown below. I want to ask, if this code is good or am I still missing anything or doing something in a wrong way.

void * malloc ( size_t size )
{
    int *plen;
    int len = size + sizeof( size ); // Add sizeof( size ) for holding length.
    
    plen = mmap( 0, len, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, 0, 0 );
    
    *plen = len;                     // First 4 bytes contain length.
    return (void*)(&plen[1]);        // Memory that is after length variable.
}

void free ( void * ptr )
{
    int *plen = (int*)ptr;
    int len;
    
    plen--;                          // Reach top of memory
    len = *plen;                     // Read length
    
    munmap( (void*)plen, len );
}
greybeard
  • 2,249
  • 8
  • 30
  • 66
MetallicPriest
  • 29,191
  • 52
  • 200
  • 356
  • 3
    I would suggest making a basic test program that runs these a bunch of times with a bunch of different parameters and running it through Valgrind to see if any issues pop up. – Dan Fego Dec 12 '11 at 14:16
  • Belongs on http://codereview.stackexchange.com ? – Paul R Dec 12 '11 at 14:34
  • @Paul: still having trouble navigating all the different sub forums, from the one you mention to, say, SO's ubuntu forum. Could you kindly advise how/where to get a list of them? – gnometorule Dec 12 '11 at 14:43
  • @gnometorule: click on the StackExchange word right at the top of the page on the left - this will pop up a window - then click on "All Sites" – Paul R Dec 12 '11 at 14:54

5 Answers5

18

Some observations:

  • You assume that int and size_t have the same size. If you want to store a size_t value at the head of the allocation, then why don't you just do that? Why introduce int?
  • This will very likely be quite inefficient, both in terms of memory usage and speed. There is significant overhead to mmap(), and typically allocations cannot be smaller than a "page". Most real allocators try to avoid calling OS-level functionality on every malloc(), in various ways.
  • If mmap() fails, it will return MAP_FAILED, and so should malloc(). Thus, you need to test for that before de-referencing the pointer returned by mmap().
  • Calling free(NULL) should be a valid thing to do; with your implementation it will very likely cause a crash since you don't NULL-check the argument before assuming it's valid.
unwind
  • 391,730
  • 64
  • 469
  • 606
  • 3
    Very often, in many C programs, the `malloc`-ed size is much smaller than a page, so you have to keep several memory chunks in one (or several consecutive) pages. For instance when you are `strdup`-ing some name or human language word, the typical size is one or a few dozen bytes, but the typical page size is 4Kbytes! You also have to accept large `malloc` requests and handle them specially. – Basile Starynkevitch Dec 12 '11 at 14:20
  • 2
    @Basile: That said, my current PC has 4096 times as much memory as my first PC allowed to some programs. So it's not out of the question that toy programs could run with this allocator, just don't try linking Firefox against it. What you *really* want, though, is *two* pages per paltry dozen-byte allocation, aka "guard page after" ;-) – Steve Jessop Dec 12 '11 at 14:33
  • Yes, but the current software on your PC is using perhaps five thousand more memory than what was used before. RAM is like disk space, it tends to be full.... – Basile Starynkevitch Dec 12 '11 at 14:35
  • @Basile: My RAM is not full: current physical memory usage is 50%, most of which can be paged out to the swap file. I say again, the size of program that can be run on my PC with this absurd allocator is roughly comparable to the size of program that could be run in conventional memory on my first PC. That doesn't make this a *good* allocator, just one that's better than completely useless. – Steve Jessop Dec 12 '11 at 14:39
  • @Basile: indeed, assuming you only hook user calls to `malloc` and not any hidden allocation that goes on in the standard libraries, your `malloc` conforms to the letter if not the spirit of the standard. MetallicPriest's is much more useful, though, since his does support programs with modest memory requirements (say, a few tens of thousand blocks allocated at any one time). – Steve Jessop Dec 12 '11 at 14:45
  • 1
    @unwind: `mmap` will return `MAP_FAILED` when it fails rather than `NULL`. – rpetrich May 02 '15 at 23:58
  • @rpetrich D'oh! Thanks, that was sloppy of me. Fixed. – unwind May 04 '15 at 07:00
10

You should, at least.

  • Check if mmap fails
  • store a size_t for the size, not an int. They might be different.
  • return suitable aligned memory, the memory here seems to be 4 byte aligned (as you add an int, presumably 4 bytes, to the page aligned data from mmap. This means storing values that need larger alignment in the returned memory, e.g. a double incurs a performance penalty, or outright crashes on some architectures.
  • Handle NULL being passed to free (it's supposed to be a no-op)

For a malloc implementation, i'd also expect it to at least have rudimentary debugging support, e.g. try to detect double frees, try to detect free'ing invalid pointers, list memory not free'd etc.

Keep in mind your malloc implementation here can be extremely wasteful. If you malloc 10 bytes , you'll end up allocating 1 page (4096 bytes), that all have to be mapped to physical memory, with the remaining 4082 bytes being unused.

Josh
  • 10,961
  • 11
  • 65
  • 108
3

Read also c malloc on wikipedia and study carefully some real malloc implementations like Doug Lea's malloc. There is a lot of litterature on the subject, e.g. by Wolfram Gloger and many many others.

@MetallicPriest: you really should explain much much more what you are doing (I am not very optimistic for your work, but you'll probably learn a big lot!)

Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547
3

You shouldn't call mmap each time you call your custom malloc. As stated before there is a huge overhead doing this.

You should create a large sized shared memory (4K for example) and your malloc function will only have to return pointers in the range of the memory you alocated.

Then if you get short on memory you create an other 4k of shared memory.

You'll just have to count how much space you use and keep track of your references and that way freeing memory is much easier.

puikos
  • 300
  • 1
  • 4
2

And you also should use -1 instead of 0 as the fd argument of mmap.

mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0)

MAP_ANONYMOUS

The mapping is not backed by any file; its contents are initialized to zero. The fd and offset arguments are ignored; however, some implementations require fd to be -1 if MAP_ANONYMOUS (or MAP_ANON) is specified, and portable applications should ensure this. The use of MAP_ANONYMOUS in conjunction with MAP_SHARED is supported on Linux only since kernel 2.4.

remus
  • 59
  • 1
  • 4