Unable to properly free malloc of another malloc

Question

Here's the snippet with issues.

int main()
{
    char** RESERV = (char**)malloc(sizeof(char*)*4);
    printf("%i, %i, %i, %i, %i", **RESERV, *RESERV, RESERV, &**RESERV, sizeof(char*));
    int i;
    for(i = 0; i < 4; i++)
    {
        RESERV[i] = (char*)calloc(sizeof(char),16);
        RESERV[i][15] = '\0';
    }
    for(i = 0; i < 4; i++)
        RESERV[i]="Iambananananananananananana";
    for(i = 0; i < 4; i++)
        printf("\r\n>%i<", RESERV[i]);
    for(i = 0; i < 4; i++)
    {
        printf("\r\n<%i>", (RESERV[i]));
        free(RESERV[i]);
    }
    free(RESERV);
}

This code free() is working fine in 32 bit , but somehow crashes horribly in 64 bit mode.

In my main program I've omitted freeing the members of the char** causing unexptected behavior every now and then, which I obviously do not want.

I've tried playing around with addresses and pointers, even tried

free(RESERV+(i*sizeof(char*))

Which failed too. Can someone clarify what I'm doing wrong?

Standard Warning : Please [do not cast](http://stackoverflow.com/q/605845/2173917) the return value of `malloc()` and family in `C`. — Sourav Ghosh, May 11 '15 at 13:23
Let's start with what you're trying to achieve, looks like you're trying to allocate a two dimensional array - the equivalent of char[4][16]. is that correct? — Ishay Peled, May 11 '15 at 13:27
Well, this is kind of the small form, a more average example is more like char[300][150]. But overall yes, 2d char array. — Nos, May 11 '15 at 13:28
This does not create a 2D array, [see this](http://stackoverflow.com/questions/30117161/why-do-i-need-to-use-type-to-point-to-type). — Lundin, May 11 '15 at 13:39
Pay attention to compiler warnings. Don't ignore them! Try to fix them, instead of ignoring. — Spikatrix, May 11 '15 at 13:54
There are no compiler warnings, I'd wish there were any. I can't stand warnings. I tried Lundin's link but I get weird behavior with printf if I print %s loop this. -Snip- Code looks awful if I paste it here. It's pretty much the same code, but with the initialisation replaced with the help from Lundin's link. I allocate 15 f's and add a '\0' at the end, but I get 27, 23, 19 and 15 f's with the last one being correct. — Nos, May 11 '15 at 13:59
"*There are no compiler warnings*" -- Really? If so, you have to increase the warning level of your compiler. Include warning flags. If GCC, include the `-Wall` and maybe `-Wextra` flags. — Spikatrix, May 11 '15 at 14:02
Flags are set and I still don't get any warnings. I'm using DevCPP with the TDM-GCC 32/64 bit debug mode. — Nos, May 11 '15 at 14:09
What?? It will sent warnings. `printf("%i, %i, %i, %i, %i", **RESERV, *RESERV, RESERV, &**RESERV, sizeof(char*));` has many problems. And did you use `strncpy`? That would give weird results when printing using `%s`. If so, NUL-terminate *after* calling `strncpy` — Spikatrix, May 11 '15 at 14:12
I do not use strncpy, I for loop 16 times, the first 15 times allocating an f and allocating '\0' at the end. I do not use strncpy as I read a lot of data from large files and need to do "stuff" with them ( remove, add, move, etc. ). I seriously do not get any warnings. Compilation results... -------- - Errors: 0 - Warnings: 0 — Nos, May 11 '15 at 14:14
I cannot understand what is wrong. If you need help, i'll have to see the code. Post a new question including the relevant parts of the code and I will be happy to help you. — Spikatrix, May 11 '15 at 14:17
I'd rather not as I don't want to appear spammy with questions, but I don't have much of a choice if I want to properly show my code. — Nos, May 11 '15 at 14:20
Oh well, I can't post for another half an hour, perhaps I can elaborate if I'm not clear enough? — Nos, May 11 '15 at 14:34
I probably interpret the post wrong, or the post is wrong. Y=X and X=Y, I threw it in the debugger and it told me one string in the malloced 2d array was 4 members long with Y = 4 & X = 16. Yeah the first one. — Nos, May 11 '15 at 14:41

Sourav Ghosh · Accepted Answer · 2015-05-11T13:46:10.977

3

In your code

 RESERV[i]="Iambananananananananananana";

creates the problem. It overwrites the memory allocated by malloc(). Thus,

You face memory leak, because the malloc()ed pointer is lost.
You cannot call free() with the changed pointer. It invokes undefined behaviour.

Solution:

In C, you don't assign strings, instead, you can use strcpy() to get your work done.

Notes:

even in case of strcpy() you cannot use "Iambananananananananananana". In this case, it will create memory overrun as destination does not have enough memory to hold it completely.
Use proper format specifiers. in your printf() statements, most of the arguments to %i are not of type int. For pointer type arguments, you should be using %p, atleast. Otherwise, it will be UB.

edited May 11 '15 at 13:46

answered May 11 '15 at 13:24

Sourav Ghosh

133,132
16
183
261

I'm wondering why 32 bit doesn't care? – Nos May 11 '15 at 13:27
2

@Nos it's not about 32 bit or 64 bit, it's about [undefined behaviour](https://en.wikipedia.org/wiki/Undefined_behavior). – Sourav Ghosh May 11 '15 at 13:28
3

@Nos Well, if it _worked_, I'll consider ___bad luck___. :-) – Sourav Ghosh May 11 '15 at 13:31
Alright, while we're on string copying anyways, while copying chars one for one is another correct way of doing it? – Nos May 11 '15 at 13:33
@Nos yeah, tha'll be ok, because, you'll be copying values in that case. :-) – Sourav Ghosh May 11 '15 at 13:34
I suggest mentioning about other things that cause UB. OP has used `%i` but arguments don't match `int*`, which `%i` expects. – Spikatrix May 11 '15 at 13:38
@CoolGuy Thanks Updated, and I believe you meant `int`, not `int *`, right? – Sourav Ghosh May 11 '15 at 13:47
Yes. I meant to say `int`, not `int*`. Got mixed up when thinking about `%p` and stuff. :) – Spikatrix May 11 '15 at 13:51

score 2 · Answer 2 · edited May 12 '15 at 09:07

2

RESERV[i]="Iambananananananananananana";

This is not the proper way to populate this string after allocating it. You are overwriting the value of that pointer.

strncpy(RESERV[i], "Iambananananananananananana", 15);
RESERV[i][15] = 0;

This is what you're looking for.

Notice, too, that you are apparently to assign something into that pointer that is larger than the memory that you have allocated.

edited May 12 '15 at 09:07

Spikatrix

20,225
7
37
83

answered May 11 '15 at 13:26

David Hoelzer

15,862
4
48
67

I wrote a larger string to see if it'd overflow to the other members, but I confess about the string allocation part. – Nos May 11 '15 at 13:32
Experimentation is good. ;) I saw that you were printing out the pointer values. Personally, I find them much more meaningful in hex (%x) rather than decimal. – David Hoelzer May 11 '15 at 13:34
This will cause more problems as `strncpy` does not NUL-terminate `RESERV[i]` – Spikatrix May 11 '15 at 14:16
@CoolGuy Ah, you're right. I should have caught that in this case or used an `snprintf`. – David Hoelzer May 11 '15 at 14:22

Ed Heal · Answer 3 · 2015-05-11T15:08:10.977

0

The line

 RESERV[i]="Iambananananananananananana";

is incorrect - You need to do

 strncpy(RESERV[i], "Iambananananananananananana", 15);
 RESERVp[i][15] = 0;

edited May 11 '15 at 15:08

answered May 11 '15 at 13:27

Ed Heal

59,252
17
87
127

Wouldn't this cause more problems as `strncpy` does not NUL-terminate `RESERV[i]`? – Spikatrix May 11 '15 at 14:10
@CoolGuy - Good point - edited answer – Ed Heal May 11 '15 at 15:08

Unable to properly free malloc of another malloc

3 Answers3