CODEWITHSUNDEEP
cmemory-managementmemory-leaks
conradkleinespel
conradkleinespel

Reputation: 6987

Memory leak working with C strings

I'm trying to build a str_replace function in C (so as to learn C). To make things a little easier, I've decided to create two helper functions, one of which has the following prototype:

char * str_shift_right(const char * string, char fill, int32_t n);

It takes a string and adds the character fill at the nth position in the given string. Here is the full code:

// replace the nth char with 'fill' in 'string', 0-indexed
char * str_shift_right(const char * string, char fill, int32_t n) {
    // +1 the null byte, +1 for the new char
    int32_t new_size = (int32_t) strlen(string) + 2;
    char * new_string = NULL;
    new_string = calloc(new_size, sizeof(char));

    new_string[new_size - 1] = '\0';
    int32_t i = 0;
    while (i < strlen(string) + 1) {
        // insert replacement char if on the right position
        if (i == n) {
            new_string[i] = fill;

        // if the replacement has been done, shift remaining chars to the right
        } else if (i > n) {
            new_string[i] = string[i - 1];

        // this is the begining of the new string, same as the old one
        } else {
            new_string[i] = string[i];
        }

        i++;
    }

    return new_string;
}

I wanted to make sure this function was not leaking memory, so I've tried executing the following code:

int main(int argc, const char * argv[])
{    
    do {
        char * new_str = str_shift_right("Hello world !", 'x', 4);
        printf("%s", new_str);
        free(new_str);
    } while (1);

    return 0;
}

However, when watching memory usage with the activity monitor (a Mac OSX application, for those not familiar, kind of like the Process Manager on Windows), it seems like RAM gets eaten up pretty fast and it does not become available when the program stops executing.

Is that what a memory leak is? If so, what have I done wrong? Isn't the free(new_str) call supposed to free up memory?

Thanks for your help.

Edit 1: Fixed off by one error spotted by PaulR. The problem remains.

Upvotes: 2

Views: 156

Answers (4)

Adri&#225;n P&#233;rez
Adri&#225;n P&#233;rez

Reputation: 2216

Although has been mentioned, I think it's worth to emphasize that if you suspect there is a memory leak in your program, don't make conclusions just by watch system monitors but using valgrind instead.

Download it from here:

http://valgrind.org/downloads/current.html

Upvotes: 0

Has QUIT--Anony-Mousse
Has QUIT--Anony-Mousse

Reputation: 77454

Update: how are you measuring? This should not be possible on UNIX: "[memory] does not become available when the program stops executing". Maybe you are just looking at the wrong number?!?

I don't see where there should be a memory. Try using tools such as valgrind!

The aforementioned invalid write has been discussed a number of times. Maybe you are destroying memory management information with this. I've seen C libraries use bytes just infront and/or after each allocated memory chunk for keeping track of allocated memory fragments. So maybe you are zeroing out the size this way and crippling memory management this way. So really, use valgrind and similar tools.

However, you may also want to simplify your code.

  1. no need to zero out memory that you are going to fully overwrite anyway.
  2. No need to use a loop, when you know the positions already.
  3. You can also copy the trailing \0.
  4. Actually do use size_t, which is unsigned. Your code above will not warn if fill is negative. With size_t it must not be negative.

So this should be sufficient (I did not compile test this - you may want to):

char* str_shift_right(const char* string, char fill, size_t n) {
  size_t len = strlen(string);
  char* new_string = malloc(len + 2, sizeof(char));
  memcpy(new_string, string, n);
  new_string[n] = fill;
  memcpy(new_string + n + 1, string + n, len - n + 1);
  return new_string;
}

The last memcpy will also copy the trailing \0 here. Overall, the C compiler may optimize this code better, memcpy is usually quite well taken care of. Plus, it actually is much easier to read.

Upvotes: 2

Basile Starynkevitch
Basile Starynkevitch

Reputation: 1

free is not supposed to release memory to the system (e.g. using munmap(2), because malloc usually prefer to re-use previously free-d memory instead of acquiring it from the kernel (e.g. using mmap)

The intuition is that syscalls managing memory address space(i.e. mmap & munmap) are quite expensive, so most malloc & free implementation try to reuse memory internally, when possible.

If you suspect a memory leak, try valgrind (recent versions have been ported to MacOSX). Of course compile your code with debugging information and all warnings (e.g. gcc -Wall -g or perhaps clang -Wall -g)

Upvotes: 2

thejh
thejh

Reputation: 45568

it seems like RAM gets eaten up pretty fast and it does not become available when the program stops executing.

Which RAM usage are you looking at? Total RAM usage in the system?

If so, what you're seeing is probably the memory that your terminal uses – every character your program prints out will be stored in RAM by the terminal (although it'll probably start dropping stuff at a certain limit). Try it again, but this time, prevent the output from showing up in the terminal:

./program > /dev/null

As a general rule, no matter how much memory you're leaking, it'll always be freed automatically when your program is terminated. And I can't spot any leaks in your program.

Upvotes: 6

Related Questions