C char array, pointers, malloc, free

Question

I am trying to understand pointers and I've got this simple example

void third(char ****msg) {
    ***msg = malloc(5 * sizeof (char));
    printf("\nthe msg in third is :%s ", ***msg);
    strcpy(***msg, "third");
    printf("\nthe msg in third after is: %s", ***msg);
    // free(***msg);
}

void change(char***msg) {
    **msg = malloc(5 * sizeof (char));
    printf("\nthe msg in change is :%s ", **msg);
    strcpy(**msg, "change");
    printf("\nthe msg in change after is: %s", **msg);
    third(&msg);
    //   free(**msg);
}

void test(char ** msg) {
    *msg = malloc(5 * sizeof (char));
    printf("\n the msg in test is: %s", *msg);

    strcpy(*msg, "test");
    printf("\nthe msg in test after is: %s\n", *msg);

    change(&msg);
    free(*msg);
}

int main(int argc, char** argv) {

    char * msg;
    test(&msg);
    printf("\nthe msg back in main is: %s", msg);
}

I could say it is working fine, but could you tell me when and how I need to free the allocated memory, because if I remove the // from functions change and third and run it I am having errors. And is there a way to get the content of the message in the first print statement of each function - see the otuput:

the msg in test is: 
the msg in test after is: test
the msg in change is :0�� 
the msg in change after is: change
the msg in third is :P�� 
the msg in third after is: third
the msg back in main is:

Is there a way to get the msg in change is : test and then the msg in third is : change

simple example? with four starts? You're way beyond becoming a t(h)ree-star programmer. — Sourav Ghosh, Nov 30 '16 at 14:00
WOW! You are the first 4-star C programmer I see! You are aware being a 3-star C programmer is not a compliment already, are you? — too honest for this site, Nov 30 '16 at 14:00
*** Error in .................. double free or corruption (fasttop): 0x00000000020e8050 *** the msg in third after is: third RUN FINISHED; Aborted; real time: 0ms; user: 0ms; system: 0ms — John Richards, Nov 30 '16 at 14:01
Use of not initialize variable result in Undefined Behavior (every `printf` with `the msg in xxx is :%s`). And for `change` and `third` you allocate 5 char but copy respectively 7 and 6 => UB — Garf365, Nov 30 '16 at 14:02
@SouravGhosh: What is a **tree**-star programmer? Someone living on a tree? ;-)) — too honest for this site, Nov 30 '16 at 14:02
@Olaf edited...but it's true, if I keep writing code like this, the safest place (to avoid the maintainer) would be a tree-house only. ;-) — Sourav Ghosh, Nov 30 '16 at 14:04
Remove change() and third() and start analyzing from the beginning. — MayurK, Nov 30 '16 at 14:05
We have a phrase here which literaly translates to "putting me on a palmtree" which means "aggravates me". That just came into mind ... — too honest for this site, Nov 30 '16 at 14:06
This program doesn't need more than 2 levels of indirection. There's no need to keep piling them, just pass the pointer-to-pointer on to the next function. — Lundin, Nov 30 '16 at 14:09
@Lundin do you meen this change(*&msg); void change(char**msg) — John Richards, Nov 30 '16 at 14:15

score 2 · Accepted Answer · answered Nov 30 '16 at 14:24

Just forget about that program, there's just too much wrong with it:

You don't need multiple levels of indirection. Two levels is enough. Simply pass on the pointer-to-pointer to the next function, if that function needs to change the address.
You try to print the string before it has been initialized.
You try to use the string after freeing it.
You create memory leaks by repeatedly calling malloc without cleaning up the old contents. Use realloc instead.
You allocate incorrect amounts of memory so the arrays aren't large enough to hold the strings you strcpy into them. Also note that you need to allocate enough room for the null terminator.

And so on. Here's a fixed version:

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

void third(char** msg) {
    const char str[] = "third";

    *msg = realloc(*msg, sizeof(str));
    printf("the msg in third is :%s\n", *msg);

    strcpy(*msg, str);
    printf("the msg in third after is: %s\n", *msg);
}

void change(char** msg) {
    const char str[] = "change";

    *msg = realloc(*msg, sizeof(str));
    printf("the msg in change is :%s\n", *msg);

    strcpy(*msg, str);
    printf("the msg in change after is: %s\n", *msg);

    third(msg);
}

void test(char** msg) {
    const char str[] = "test";

    *msg = malloc(sizeof(str));
    printf("the msg in test is just garabage at this point, no need to print it.\n");

    strcpy(*msg, str);
    printf("the msg in test after is: %s\n", *msg);

    change(msg);
}

int main(int argc, char** argv) {

    char* msg;
    test(&msg);
    printf("the msg back in main is: %s\n", msg);

    free(msg);
}

Excellent ! Now I understand. Does this apply for other data types e.g. int or int [] ? — John Richards, Nov 30 '16 at 14:26
@JohnRichards The data type of the array doesn't matter, but `char` arrays used as strings is a special case, since they need to be null terminated. When declaring `const char str[] = "test";` you get an array of 4 characters + 1 null terminator. That's why `sizeof(str)` works. — Lundin, Nov 30 '16 at 14:56

score 1 · Answer 2 · edited May 23 '17 at 12:08

In real life, avoid use as much "star" (ie number of indirection). Also your code has several issue involving undefined behavior (UB)

void third(char ****msg) {
    ***msg = malloc(5 * sizeof (char));
    printf("\nthe msg in third is :%s ", ***msg);// Don't do that, ***msg is not intialize => UB
    strcpy(***msg, "third"); // Don't do that => ***msg has 5 bytes, and you copy 6 char => UB
    printf("\nthe msg in third after is: %s", ***msg);
    // free(***msg);
}

void change(char***msg) {
    **msg = malloc(5 * sizeof (char));
    printf("\nthe msg in change is :%s ", **msg);// Don't do that, **msg is not intialize => UB
    strcpy(**msg, "change"); // Don't do that => **msg has 5 bytes, and you copy 7 char => UB
    printf("\nthe msg in change after is: %s", **msg);
    third(&msg);
    //   free(**msg);
}

void test(char ** msg) {
    *msg = malloc(5 * sizeof (char));
    printf("\n the msg in test is: %s", *msg); // Don't do that, *msg is not intialize => UB

    strcpy(*msg, "test");
    printf("\nthe msg in test after is: %s\n", *msg);

    change(&msg);
    free(*msg);
}

int main(int argc, char** argv) {

    char * msg;
    test(&msg);
    printf("\nthe msg back in main is: %s", msg); //UB => you use freed variable
}

After correcting this multiple sources of UB, it still a bad design: use of free function

According fact that you use pointer of pointer of... when you do a malloc inside a function, you also modify pointer of caller. Try run this and see:

void first(char **t)
{
    *t = malloc(5*sizeof(char));
}

int main(int argc, char * argv[])
{
    char * t = NULL;
    printf("%p\n", t);
    first(&t);
    printf("%p\n", t);
    return 0;
}

it produce (demo):

(nil)
 0x995f008 (or another address)

So when you free in test, you free memory allocated in third

Finally, as already mentioned in commentary, only 2 stars is enough:

void third(char **msg) {
    free(*msg); // Free before allocate and change address stored in pointer
    *msg = malloc(6 * sizeof (char));
    strcpy(*msg, "third");
    printf("\nthe msg in third after is: %s", *msg);
}

void change(char **msg) {
    free(*msg); // Free before allocate and change address stored in pointer
    *msg = malloc(7 * sizeof (char));
    strcpy(*msg, "change");
    printf("\nthe msg in change after is: %s", *msg);
    third(msg);
}

void test(char ** msg) {
    *msg = malloc(5 * sizeof (char));

    strcpy(*msg, "test");
    printf("\nthe msg in test after is: %s\n", *msg);

    change(msg);
}

int main(int argc, char** argv) {

    char * msg;
    test(&msg);
    printf("\nthe msg back in main is: %s", msg);
    free(msg); // Deallocate memory ONLY when you don't need it anymore
    msg = NULL; // Good practice, set to NULL freed pointer to inform that no more memory are allocated
}

Alright, I understand. Then what about free-ing the memory. Is the free in test enough? — John Richards, Nov 30 '16 at 14:10

C char array, pointers, malloc, free

2 Answers2