0

After running this (it compiles fine), I get "double free or corruption", but only specifically if I set n to be odd. There's no problem for n even, and I'm really confused...

#include<stdio.h>
#include<stdlib.h>

typedef unsigned int uint;

int main(void)
{   
    int i, j;
    uint n = 3;

    uint*** a = (uint***) malloc( n* sizeof(uint**) );
    for(i=0; i<n; i++)
    {
        *(a + i) = (uint**) malloc( (n)* sizeof(uint*) );
        for(j=0; j<n; j++);
        {
            (*((*(a + i))+j)) = (uint*) malloc(1 * sizeof(uint));
        }
    }

    for(i=0; i<n; i++)
    {
        for(j=0; j<n; j++);
        {
            free (*((*(a + i))+j));
        }
        free (*(a + i));
    }
    free(a);
}
Florian Wartelle
  • 13
  • 1
  • Don't cast `malloc()`. – melpomene May 28 '18 at 14:32
  • What's with the paren orgy in `(*((*(a + i))+j))`? Why not just write `*(*(a + i) + j)`? – melpomene May 28 '18 at 14:33
  • 1
    why allocating 1 element? – Jean-François Fabre May 28 '18 at 14:33
  • 3
    I can’t be bothered to parse horrible expressions like `(*((*(a + i))+j))` - it may or may not be correct - why not just use `a[i][j]` ? – Paul R May 28 '18 at 14:34
  • 5
    @melpomene it would help if you at least link to the question discussing why you [shouldn't cast](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). Also don't be a [three star programmer](http://wiki.c2.com/?ThreeStarProgrammer). – Kami Kaze May 28 '18 at 14:34
  • 1
    @PaulR For the record, it is correct, and aye, it's vile. – George May 28 '18 at 14:35
  • 1
    It is necessary for there to be exactly one call of `free()` for every call of `malloc()`. One simple way in your code is to do the `free()` calls in reverse order compared with their `malloc()` calls. Also `malloc()` does not need to be cast and, lastly, there is no need to be cute with pointer syntax ..... `(*((*(a + i))+j))` is expressed much more simply as `a[i][j]`. – Peter May 28 '18 at 14:38
  • it is quite ironic if you need at least three times as much memory for the pointers then for the data. – Kami Kaze May 28 '18 at 14:49
  • Thanks to all that recommend to better my horible expressions, I'll do my best. Advice about proper form is rare in the context in which I work. – Florian Wartelle May 28 '18 at 15:44

1 Answers1

6

Here's your problem:

    for(j=0; j<n; j++);
//                    ^
    {
        (*((*(a + i))+j)) = (uint*) malloc(1 * sizeof(uint));

This extra ; at the end of the for line means you have a loop with an empty body:

    for(j=0; j<n; j++)
        ;

... followed by a single assignment:

        (*((*(a + i))+j)) = (uint*) malloc(1 * sizeof(uint));

At this point j == n (because of the preceding loop), so you're writing out of bounds.

The same bug exists further down in your free code (copy/paste?).

melpomene
  • 84,125
  • 8
  • 85
  • 148