Baffling problem : can't find source of leaks

Hey all,

My friend and I are on an assignment for our programming school. The assignment is to create a function which will read & return lines from a file, one line per call, using a static variable. It should return NULL in case of invalid fd, or nothing more to read. It's prototyped like this :

Code:

---

char    *get_next_line(int fd)

---

I was able to turn in the assignment and get full grades on the first try. My friend has failed twice already, because of supposed leaks, and we can't figure out where they're coming from.

Our school has an automated program which runs extensive tests on our own programs to grade us. It is that automated program which detects leaks. Unfortunately we have no access to the code or the files it's testing with. We can only take it at its word.

We've used two different testers made by other students, both use valgrind, we detect no leaks. One of them tests with a combination of different files (empty, empty with newline, one line, big lines, etc...), the other tests with massive randomly generated files.

We've tested with really big text files, really big non-text files (my program didn't handle binary and it passed, so it's not needed). No leaks.

We cleaned up his code (it was a bit wonky but it worked) before sending it a second time, the exact same leaks were found on the exact same tests.

I usually have a good eye for these things. I don't give up. I was 100% sure we missed something. There's no way the grading program is wrong. But I admit defeat, I have NO idea where it's going wrong. We've tested 99.9% of cases, and the grading program seems to find leaks on even the first few tests it runs (which should be basic tests).

We need help. We're completely lost here. Of all the students we've asked for help, nobody could find what was going wrong. I'm starting to believe the grading program ........ed up somewhere.

Is there anything else than valgrind which could find leaks valgrind can't?

Does anyone have any idea what kind of weird scenario could cause such a simple program to leak?

Here is the code:

get_next_line/get_next_line.c at master * m0d1nst4ll3r/get_next_line * GitHub

get_next_line/get_next_line_utils.c at master * m0d1nst4ll3r/get_next_line * GitHub

Thank you!

Edit: thinking about it, the mallocs are not all protected, but they still wouldn't cause leaks or cause any catastrophic behavior. I don't think the grading program tests that and if it did, certainly not on the first few tests.

So can you post your valgrind log output?

Can you control the options you give to valgrind?

In get_line, I noticed:

Code:

---

line = ft_substr(*s, 0, i);
tmp = *s;
*s = ft_substr(tmp, i, ft_strlen(tmp) - i);
free(tmp);
return (line);

---

ft_substr returns a string allocated by malloc. The string input to get_line is from read_line, which itself calls a function that uses malloc.

So, what you have here are a net of three malloc calls and only one call to free. It's possible that the caller of get_next_line is responsible for calling free, but that still leaves one unmatched call to malloc, and thus a memory leak. It's not a big deal because a subsequent call to get_next_line will normally call free, but the very last call will leave it allocated until the end of the program. Perhaps a solution is to call free in get_next_line before returning a null pointer when it is clear that there's nothing more to read yet the buffer is empty.

EDIT:
I originally posted this thinking that your friend mixed up using s versus using the return value of get_line as the return value of get_next_line, then I realised that s was being used to save the unused portion of the buffer. Your friend probably should use a more descriptive variable name.

One of the malloc calls can be eliminated by simply inserting a null character at the right place instead of calling ft_substr. The corresponding free will be removed though, so this does not fix the memory leak.

Also, instead of joining strings by finding the end of the first string from the start, it's more efficient to just join the second string starting from the end of the first, since your friend is already computing the string lengths so the location of the end of the first is already known.

@laserlight, It is presumably the function-user's responsibility to free the returned lines.

@Modin, Can you post a link to your code so I can compare them?

@Salem
valgrind tells us 0 loss whatsoever, BUT it was telling us there was 1 byte still reachable which we figured was fine.
It just clicked for me that it's probably not. That's probably what caused us so much trouble. That one byte we never free once the program's started.

@laserlight
I agree with s being badly named! This is a common mistake for us beginners.
You pointed out something that wasn't the problem, but you helped us figure it out nonetheless. We think we know what's going on. The "s" static variable that we malloc'd while reading the file still has a malloc'd empty string inside it once we're done reading the file. We think the grading program wants us to free *everything* once we're done parsing through a file. That single '\0' byte was griefing us from the start.
Also agreed on the computing efficiency.

@john.c
Here is mine GitHub - m0d1nst4ll3r/42_get_next_line
I'm doing it quite differently, I use structures and chained lists. This is to handle multiple file descriptors. He handles them (in the _bonus files) by using a char ** as static and allocates 1024 slots inside, because supposedly that's the maximum amount of file descriptors we'll ever see.
The reason I passed the tests is probably that I decided (totally on a whim) to free a list element when its file descriptor is done being useful (when we reach end of file). It seemed cleaner to me but I had no idea it was *necessary* since it's not specified in the assignment.


We just added

Code:

---

free (s);
s = NULL;

---

in get_next_line before the second return (NULL), we should be fine now. We'll confirm tomorrow but we feel pretty confident that that was it!

Thank you all for your help. Sorry it wasn't anything crazy interesting after all :'(

Quote:

---

Originally Posted by john.c

@laserlight, It is presumably the function-user's responsibility to free the returned lines.

---

It is impossible for the caller of get_next_line to call the equivalent of free(s) as s is never returned.

Quote:

---

Originally Posted by Modin

You pointed out something that wasn't the problem, but you helped us figure it out nonetheless. We think we know what's going on. The "s" static variable that we malloc'd while reading the file still has a malloc'd empty string inside it once we're done reading the file.

---

That's exactly what I was talking about, hence I mentioned that "the very last call will leave it allocated until the end of the program" and suggested "call free in get_next_line before returning a null pointer when it is clear that there's nothing more to read yet the buffer is empty". Remember, s in get_line points to s in get_next_line, so *s in get_line is an alias for s in get_next_line. Therefore, this line in get_line that I mentioned:

Code:

---

*s = ft_substr(tmp, i, ft_strlen(tmp) - i);

---

results in a malloc call within ft_substr that allocates to s in get_next_line. s in get_next_line is (or rather points to) "the buffer".

You're absolutely right and I apologize for skimming through your answer way too fast. I think I was too excited that I'd just figured it out, thanks to you! So, many thanks and also, wow, you saw the mistake so clearly without knowing our code beforehand. Kudos :)