Why does freeing a dynamically created 2D long long int array gives SIGTRAP in c?

Question

I am creating a simple 2D long long int array dynamically in C, but freeing the array gives SIGTRAP ? I am really confused.The same code for int works perfectly fine. Why using it with long long int gives SIGTRAP ?

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

int main()
{
int i,j;
int n;
scanf("%d",&n);
long long int** ptr=(long long int**)calloc(n,sizeof(long long int*));
for(i=0;i<n;i++)
ptr[i]=(long long int*)calloc(n,sizeof(long long int));

for(i=1;i<=n;i++)
for(j=i;j<=n;j++)
ptr[i-1][j-1]=i+j;

free(ptr[i]);
free(ptr);
return 0;
}

Standard Warning : Please [do not cast](http://stackoverflow.com/q/605845/2173917) the return value of `malloc()` and family. — Sourav Ghosh, Mar 09 '15 at 12:19
@ShikharSrivastava Hello. After the latest edit, does the problem persist? — Sourav Ghosh, Mar 09 '15 at 12:31
can stack overflow not just parse c code snippets for parentheses preceding a malloc and automatically add the appropriate ubiquitous comment? — bph, Mar 09 '15 at 12:53
@SouravGhosh yes the problem still persist please have a look at the final edit — Shikhar Srivastava, Mar 09 '15 at 13:06
There ought to be `n + 1` calls to `free()`: **Exactly which** call to `free()` causes the crash? — alk, Mar 09 '15 at 13:15
Also you are expected to properly indent your code, at least in the case you are expecting us to read it. — alk, Mar 09 '15 at 13:17
@alk sir, IMHO, in this particular case, the very first call. as `i` is `n+1` then. (`i` value held after `for` loop):-) — Sourav Ghosh, Mar 09 '15 at 13:25
@SouravGhosh: Yes sure, for the code currently shown (as per edit 2). I however assume this isn't the "real" code. — alk, Mar 09 '15 at 13:29
@juanchopanza sir, isn't the `for` loop condition here is `i<=n`? so the failure case in `for` loop has to be `i>n`, right? and here we've use `i++`, so... — Sourav Ghosh, Mar 09 '15 at 13:32
@SouravGhosh Yes, you're absolutely right. I must be sleep-typing. — juanchopanza, Mar 09 '15 at 13:42
@juanchopanza I would like to blame OP for using non-conventional indexing scheme. — Sourav Ghosh, Mar 09 '15 at 13:44

score 1 · Answer 1 · edited May 23 '17 at 11:43

1

in ptr, you're allocating memory for n times sizeof (long long int *), but you're looping for a hardcoded value usning for(i=0;i<50;i++).

In case, n is less that 50, you're accessing out-of-bound memory by saying ptr[i]. Results in undefined behaviour.

That said, you should always check the return value of scanf() to ensure proper input, otherwise, as the code is currently written, in case scanf() fails, you'll face UB due to use of uninitialized local variable n.

Note: You need not and do not cast the return value of malloc() and family.

EDIT:

This time, the issues are

accessing out-of-bound memory, in free(ptr[i]); without a reset of i value.
Resulting Memory leak due to improper usage of index.

edited May 23 '17 at 11:43

Community

1
1

answered Mar 09 '15 at 12:22

Sourav Ghosh

133,132
16
183
261

1

This answer is no longer valid after the last edit of the question :( – David Ranieri Mar 09 '15 at 12:26
@AlterMann Right you are. but after the latest edit, should there be any issue(s)? – Sourav Ghosh Mar 09 '15 at 12:30
Now his code is fine, there must be something wrong with the input, OP, check the result of `scanf` – David Ranieri Mar 09 '15 at 12:34
Though I agree with " not cast the retval of malloc and friends" I still wonder, why most of the online tutorials, which I was reading, mostly did this. And for sure, the authors are no dummies, – icbytes Mar 09 '15 at 12:50
@icbytes, it's a guess ... They are trying to write code compilable in both languages, C and C++ – David Ranieri Mar 09 '15 at 12:58
sorry to bother everyone now the code is finally edited which gives SIGTRAP for unknown reasons , please help ? – Shikhar Srivastava Mar 09 '15 at 13:05

score 0 · Answer 2 · answered Mar 09 '15 at 14:22

Here is your code with correct indentation for the structure.

int main()
{
    int i,j;
    int n;
    scanf("%d",&n);
    long long int** ptr=(long long int**)calloc(n,sizeof(long long int*));
    for(i=0;i<n;i++)
        ptr[i]=(long long int*)calloc(n,sizeof(long long int));

    for(i=1;i<=n;i++)
        for(j=i;j<=n;j++)
            ptr[i-1][j-1]=i+j;

    free(ptr[i]);
    free(ptr);
    return 0;
}

As you can see, the line free(ptr[i]); is only executed after the loop through all the is has exited. The loop exits when i == n + 1. ptr has been allocated with n blocks of memory indexed from 0 to n - 1. You are trying to free a garbage pointer that is after the end of the array.

I suspect what you really want is:

for (i = 0 ; i < n ; ++i)
{
    free(ptr[i]);
}

It's a matter of style, but I prefer to always put the braces in on loop and conditional bodies even if there is only one statement in the loop.

Why does freeing a dynamically created 2D long long int array gives SIGTRAP in c?

2 Answers2