Where to free In C Code

Question

I have a working C code. It connnects 2 char arrays and prints the solution out.
But I have a dynamic memory management here (malloc) and have to free it now.
Can someone tell me where to free and why?

char * stringcat(const char * str1, const char * str2) {
    int length_1 = strlength(str1);
    int length_2 = strlength(str2);
    int length = length_1 + length_2 + 1;

    char * newstr = malloc(sizeof(char) * length);

    for(int i=0; i < length; i++)
    {
        char charToCopy = '\0';
        if(i < length_1)
        {
            charToCopy = *(str1 + i);
        } else if(i < (length_1 + length_2)) {
            charToCopy = *(str2 + i - length_1);
        }

        *(newstr + i) = charToCopy;
    }

    return newstr;
}


int strlength(char *c) {
    int count = 0;
    while(*(c + count) != '\0') {
        count++;
    }
    return count;
}


int main() {

    char surname[] = "Simon";
    char lastname[] = "Arndt";

    printf("%s\n", stringcat(surname, lastname));

    return 0;
}

Please take the [tour] and read [Ask]. There are probably existing answers to this question in these search results: https://stackoverflow.com/search?q=%5Bc%5Dwhere+to+free — jwdonahue, Jan 18 '18 at 00:19

Elliott Frisch · Answer 1 · 2018-01-18T00:07:49.203

2

You leak memory because you don't assign the result of stringcat to a char *. Do so. And then free that after you print. Or (in a trivial program such as presented here) don't worry about it, and let main exiting cause it for you.

char *name = stringcat(surname, lastname);
printf("%s\n", name);
free(name);

edited Jan 18 '18 at 00:07

answered Jan 17 '18 at 23:56

Elliott Frisch

198,278
20
158
249

don't worry about free? xdd – mariusz_latarnik01 Jan 17 '18 at 23:59
But how to free after print? – Simon Arndt Jan 17 '18 at 23:59
Like that: free(stringcat(surname, lastname)); ? – Simon Arndt Jan 18 '18 at 00:00
2

@mariusz_latarnik01 in a simple program like the OP's program there is no harm in forgetting the `free`, because the OS will free any memory left when the process is gone. I do however believes that that's bad style. For larger projects you should free the memory when you don't need it. – Pablo Jan 18 '18 at 00:04
@SimonArndt If you call it twice, then you allocate memory twice. If you `free` it as you propose there then you allocate two and free one. That's the same memory leak (plus a pointless allocation and free). – Elliott Frisch Jan 18 '18 at 00:06
@SimonArndt read our answers, we've both told you how to deal with it. – Pablo Jan 18 '18 at 00:07

Pablo · Accepted Answer · 2018-01-18T00:15:53.557

2

Usually you should free the memory at the point where you don't need it any more. For that you should store the pointer somewhere.

int main() {
    char surname[] = "Simon";
    char lastname[] = "Arndt";
    char *res;

    res = stringcat(surname, lastname); // first store it
    if(res == NULL)
    {
        fprintf(stderr, "error");
        return 1;
    }

    // then print it
    puts(res); // equivalent to printf("%s\n", res);


    free(res);

    return 0;
}

Don't forget that C-Strings are \0-terminated, that means that when you dynamically allocate memory for a string, you should allocate at least strlen(string) + 1:

char * newstr = malloc(length + 1);

if(newstr == NULL)
    return NULL;

In case of strings you don't need the sizeof(char) because it is always one.

For other types, avoid using sizeof(<type>), it's easy to make mistakes. Instead do it like this:

int *arr = malloc(10 * sizeof *arr);

The sizeof *arr will always return the correct number of bytes.

edit

You should always check for that malloc doesn't return NULL. Sure, for a simple program like this, you can skip it. But in a larger project, you definitely should check that.

edited Jan 18 '18 at 00:15

answered Jan 18 '18 at 00:01

Pablo

13,271
4
39
59

thanks. so you recomment to null res at the end? "res = NULL;" – Simon Arndt Jan 18 '18 at 00:23
@jwdonahue technically there is no guarantee that size of pointers should the same, but in practice they are. `sizeof *ptr` however will return the correct size. See https://stackoverflow.com/questions/1241205/are-all-data-pointers-the-same-size-in-one-platform-for-all-data-types – Pablo Jan 18 '18 at 00:24
1

@SimonArndt For me, it depends. There are good reasons for doing it, but in your example there is no point, because your program ends at that point anyway. See https://stackoverflow.com/a/1025604/1480131 – Pablo Jan 18 '18 at 00:27
@jwdonahue I didn't say that. If `arr` is for example a simple pointer, then `sizeof arr != sizeof *arr` (there might be types where this is true, but not in general). – Pablo Jan 18 '18 at 00:35
@Pablo, brain fart. I deleted my comment the same time you posted your response. My lesdyxia is messing with my head today. – jwdonahue Jan 18 '18 at 00:36
While not an issue in the case, always make sure you check for other return statements in the code that have might otherwise exited without freeing any malloc'ed memory. Often that "never-mind I'm leaving early," return statement can have you bleeding heap memory. – MountainLogic Jan 18 '18 at 00:40

Where to free In C Code

2 Answers2