The code works as it is supposed to, though it never frees the memory allocated by malloc().

I have tried to free memory in any place that I can, but no matter where I do it, it breaks the program. Specifically, I get a "double free or corruption error." This is more of a question as to what free() and malloc() actually do? All of the problems with free are in the main:

There is no reason to copy file_to_open out of argv, you can just useargv.
–
meagarSep 13 '11 at 4:51

Curious about incoming_string=csestrcpy2(incoming_string); Is this just a string copy function?
–
CoeffectSep 13 '11 at 4:54

2

Also, malloc(strlen(str+1)); is almost certainly wrong. You probably meant malloc(strlen(str)+1);. (And many people, including me, recommend omitting the cast - void * pointers will be implicitly converted, and it can cause potential problems if you explicitly convert it to the wrong type.)
–
Chris LutzSep 13 '11 at 4:55

csestrcopy2(incoming_string); is a string copy method that returns a string that is guaranteed to have a '/0' at the end of the line....
–
Koffeeaddict4evaSep 13 '11 at 5:39

Please post the entire code from your csestrcopy2 function because, judging by what you posted in the comments, that's part of your problem.
–
CoeffectSep 13 '11 at 6:02

7 Answers
7

In general you only have to free memory that has been reserved for you dynamically. That means if you have a statement like this:

int *my_int_pointer;
my_int_pointer = malloc(sizeof(int));

than you need to free the memory that was allocated (reserved) by malloc.
if you are unsure where to free it than just free it at the end of the program, by using free;

free(my_int_pointer);

In your file it looks like there will be memory allocated whenever there is a new line in the file you read (in the while(done==0) loop). so everytime after the if in the this loop you have to free the memory that was used by the variable.

Furthermore you need to free the memory that was allocated by for the readline variable. But as it was pointed out before you may have a memory leak there.

Hope this helps.

edit: Okay - I was already wondering about the csestrcpy function. Lets have a look at this function:

What you could however free is the src pointer in that function. but remember: the pointer cannot hold information after the underlying memory is freed! It just points to a place in memory where it should not write or read anymore.

Furthermore the function copys the string as long as there is no '\0'. What happens if there is no terminator? The function keeps on copying from some memory adresses where it should not!

tried to free to free the memory where you said and got the: free(): invalid pointer: 0xb76f5000 maybe this is unfixable
–
Koffeeaddict4evaSep 13 '11 at 5:50

Also you are correct about the read in... I have to read a file then store each line read in in as a properly sized null terminated string in an array of strings... this actually works just fine... Can I free the memory allocated for that array if i want to read the array later? If i do free it can i still read the array? How much memory can I malloc() before my program gets dead?
–
Koffeeaddict4evaSep 13 '11 at 6:03

There needs to be a call to free() for each successful call to malloc().

That doesn't necessarily mean that you need to have equal numbers of malloc() and free() calls in your code; it means that for every malloc() call that's executed when your program runs, you should call free(), passing it the pointer value you got from malloc(). malloc() allocates memory; free() tells the system that you're done with the allocated memory.

(You can almost certainly get away with not free()ing allocated memory when your program terminates, since it will be reclaimed by the operating system, but just as a matter of style and good practice you should still match malloc()s with free()s.)

Dynamic memory allocation (malloc) allocates a memory block of requested size, and returns a pointer to the start of this block.Since we have taken this block from memory so its a good practice to return this back to memory after completion of task.

Now as answer of your question , To be always on safe side you can call free function before making return.

As a general rule, for every malloc there should be a corresponding free. You cannot however free something twice (as you have noticed by now). I don't see any calls to free in your code, so it's impossible to say where your problem lies, but I noticed right away that you malloc some memory and assign it to readline inside of a loop, yet you don't call free on readline at the end of the loop, so you are leaking memory there.

if i fee it at the end of the loop then the program dies... if i free it after the loop then the program dies... if i free it at the end of the program the program dies.... it seems to go like this for any combination of frees at any point in the program...
–
Koffeeaddict4evaSep 13 '11 at 5:52

@Koffee: It certainly does not. I meant at the end of each iteration. The number of iterations is unknown, so you should be cleaning up after yourself. What if you are reading a huge file? Don't design your code to only work under the most trivial of conditions, and you should get into the habit of writing correct code anyway.
–
Ed S.Sep 13 '11 at 5:55

How do I kill the memory leak? or at least find out where it was?
–
Koffeeaddict4evaSep 13 '11 at 5:53

You cannot 'kill' a memory leak, it happens when your program terminates without freeing memory it allocated. Most Operating systems keep track of memory allocated and free on their own but there is no guarantee. You would have to restart your operating system to recover the memory. You compile your program with debugging support enabled (-g in gcc) and then use a debugger such as gdb or valgrind. valgrind even has a tool called memcheck which shows list of memories which were allocated (and where)
–
Nalin KanwarSep 13 '11 at 6:09

Awesome thanks I also found a way to do a code re-write that needs only one malloc()... I appreciate all your help!
–
Koffeeaddict4evaSep 13 '11 at 6:58

1

@Nalin: sorry, but that is incorrect. The OS does indeed keep track of memory allocated for a particular program. However when the program exits, the OS can and does reclaim all of that memory. The last OS that did not do this, iirc was Windows 98 (which is why it needed rebooting so often).
–
ChrisSep 13 '11 at 13:03

Actually, it is perfectly legal to free() a NULL pointer (e.g. returned by malloc in your last sentence). The libc will just do nothing, as specified in the standard. However, some coding guidelines may complain about doing this, as it may show that the programmer has no clear idea of what is going on with the allocated blocks and that there is possible memory leaks elsewhere.
–
calandoaSep 13 '11 at 14:10

i cannot remove that line or the array of strings never fills up thus when i try to access the array of strings later even just to print it it it gives me an array with nothing in it?
–
Koffeeaddict4evaSep 13 '11 at 5:43

also i tried to add that free and got: double free or corruption (top): 0x088113c8 and the program broke...
–
Koffeeaddict4evaSep 13 '11 at 5:43

tried both and they both give me double free or corruption (top): 0x088113c8
–
Koffeeaddict4evaSep 13 '11 at 5:45

Have edited the post. i'm sorry, the free should be just after the iterators, before the } and not after. also strings_array[count_number_of_lines]=(char*)malloc(strlen(incoming_string+1)); should be strings_array[count_number_of_lines]=(char*)malloc(strlen(incoming_string)+1)); then try freeing it like shown above - it should work
–
Jan SSep 13 '11 at 6:14

You've written free(dest) and then you are returning it! It doesn't return a string. you don't need to free the memory there. if you remove that and then free it at the end it should work fine.
–
Jan SSep 13 '11 at 8:11