Creating *** object (pointer to pointer to pointer) in C

Question

I am trying to write a function that creates a contiguous block of memory and assigns it to a 3d array. The code works in that it allows me to use the memory, and, when I use data stored in objects created with this function, the results appear correct. However, when I try to free the memory I have allocated with this function, I immediately get a glibc error. Here is the function:

void *** matrix3d(int size, int rows, int cols, int depth) {
    void ***result;

    int col_size = depth * size;
    int row_size = (sizeof(void *) + col_size) * cols;
    int data_size = (rows * cols * depth + 1) * size;
    int pointer_size = rows * sizeof(void **) + cols * sizeof(void *);
    int i, j;
    char *pdata, *pdata2;

    if((result = (void ***) malloc(pointer_size + data_size)) == NULL)
            nerror("ERROR: Memory error.\nNot enough memory available.\n", 1);

    pdata = (char *) result + rows * sizeof(void **);

    if((long) pdata % (col_size + sizeof(void *)))
            pdata += col_size + sizeof(void *) - (long) pdata % (col_size + sizeof(void *));

    for(i = 0; i < rows; i++) {
            result[i] = pdata;

            pdata2 = pdata + cols * sizeof(void *);

            for(j = 0; j < cols; j++) {
                    result[i][j] = pdata2;
                    pdata2 += col_size;
            }

            pdata += row_size;
    }

    return result;
}

It is called in this manner:

double ***positions = (double ***) matrix3d(sizeof(double), numResidues, numChains, numTimesteps);

for(i = 0; i < numResidues; i++)
    for(j = 0; j < numChains; j++)
        for(k = 0; k < numTimesteps; k++)
             positions[i][j][k] = 3.2;

free(positions);

What have I done wrong? Thank you for the help.

We cannot replay your error remotely; what is the error message you are getting? PS - casting the return from malloc is ugly as hell — tbert, Aug 15 '12 at 06:34
The error message is: *** glibc detected *** ../bin/bendingFluctuations: double free or corruption (!prev): 0x000000000186e010 ***. As far as casting the return from malloc, how else would I do it? — wolfPack88, Aug 15 '12 at 06:35
glibc detects that you're casting the return value of malloc and punishes you. — , Aug 15 '12 at 06:37
The error is probably in the code you've replaced with `....`. Can you give us complete, compilable code that replicates the error? (Or just run your code under `valgrind`.) — David Schwartz, Aug 15 '12 at 06:42
link in Electric Fence with -lefence . You may have to install Electric Fence first. It will find where you overwrite memory in many cases. — Prof. Falken, Aug 15 '12 at 06:46
@DavidSchwartz: I have added in some simple code that gives me the same error. I'm not including the original code as it is upwards of 100 lines, and would not be likely to be of much use. — wolfPack88, Aug 15 '12 at 06:49
That's enough to replicate the error. The memory allocated is too small and you are writing outside its bounds. See my answer. — David Schwartz, Aug 15 '12 at 06:53
Since you are on linux there is no excuse of not using valgrind. Compile your code with `-g` run it under valgrind and it will tell you where you access your data out of bounds. No need to ask human experts for that. A quick glance into your code shows that your code could profit from a good code review, though. — Jens Gustedt, Aug 15 '12 at 06:55

score 3 · Answer 1 · answered Aug 15 '12 at 06:35

3

What have I done wrong?

Your code is hard to follow (you're playing with pdata a lot) but 99% you're writing past the allocated space and you're messing up the bookkeeping left by glibc.

I can use the data I've written just fine. The only issue is when I try to use free.

That's because glibc only gets a chance to see you messed up when you call it.

answered Aug 15 '12 at 06:35

cnicutar

178,505
25
365
392

I thought that at first as well, but that does not seem to be the issue, since, as I mentioned, I can use the data I've written just fine. The only issue is when I try to use free. – wolfPack88 Aug 15 '12 at 06:37
1

@wolfPack88: One has nothing to do with the other. That you can use the data you've written just fine doesn't mean it's inside the block you allocated. – David Schwartz Aug 15 '12 at 06:39
Sure, but I don't understand how using free afterwards would cause an error if that is the case. My understanding of free is that it simply deallocates the memory previously allocated, right? – wolfPack88 Aug 15 '12 at 06:41
1

@wolfPack88 Where does it get the size of the allocated block ? Maybe it's stored somewhere adjacent to the allocated memory ? – cnicutar Aug 15 '12 at 06:41
1

@wolfPack88: You'd get the error in `free` because that's the first chance the memory management code has to report an error. Yes, `free` deallocates the memory previously allocated, so if you've tampered with any memory that's not yours (for example, overwriting the structures the memory manager relies on), that's when you'll find out about it. – David Schwartz Aug 15 '12 at 06:43
1

To support cnicutar's conjecture that writing past the allocated memory of pdata may corrupt the metadata used by free http://stackoverflow.com/questions/1957099/how-do-free-and-malloc-work-in-c – hhafez Aug 15 '12 at 06:47

score 1 · Accepted Answer · answered Aug 15 '12 at 06:53

1

Please excuse my dear Aunt Sally.

int data_size = (rows * cols * depth + 1) * size;

This should be:

int data_size = (rows * cols * (depth + 1)) * size;

Running the code under valgrind identified the error immediately.

answered Aug 15 '12 at 06:53

David Schwartz

179,497
17
214
278

What version of valgrind are you using? Any fancy options? Because I used it and it gave me nothing. It said 0 errors from 0 contexts, which is obviously wrong because your fix works perfectly (thanks). – wolfPack88 Aug 15 '12 at 06:57
1

No special options. The key detail was ` Address 0x4c41750 is 0 bytes after a block of size 5,904 alloc'd`. (By the way, you really shouldn't write code like this unless you really have to. Just use separate blocks.) – David Schwartz Aug 15 '12 at 07:00
I probably don't really have to in this particular case, but there are times when having a contiguous block of memory is absolutely necessary. Out of curiosity, though, why would you recommend against this type of code? It doesn't detract from anything, does it? – wolfPack88 Aug 15 '12 at 07:10
1

It's very, *very* hard to understand and debug. – David Schwartz Aug 15 '12 at 07:28

score 0 · Answer 3 · answered Aug 15 '12 at 06:44

What you are doing is one single allocation and then casting it to a tripple-pointer, meaning you have to deal a lot with offsets.

It would probably be better to a larger number of allocations:

char ***result = malloc(sizeof(char **) * rows);

for(i = 0; i < rows; i++) {
    result[i] = malloc(sizeof(char *) * cols);

    for(j = 0; j < cols; j++) {
        result[i][j] = malloc(sizeof(char) * size);

        /* Copy data to `result[i][j]` */
    }
}

When freeing, you have to free all of the allocations:

for(i = 0; i < rows; i++) {
    for(j = 0; j < cols; j++) {
        free(result[i][j]);
    }

    free(result[i]);
}

free(result);

score 0 · Answer 4 · answered Aug 15 '12 at 07:00

0

things like this are magnificant candidates to get things wrong

pdata = (char *) result + rows * sizeof(void **);

there is no reason at all to circumvent the address computation that the compiler does for you.

answered Aug 15 '12 at 07:00

Jens Gustedt

76,821
6
102
177

Creating *** object (pointer to pointer to pointer) in C

4 Answers4