get_string memory leak?
-
I am taking Harvard's CS50 class. We are learning C. They use their own library where you include cs50.h. There is a function called get_string that returns a string. I think it has a memory leak. I think this is the source code: https://github.com/cs50/libcs50/blob/main/src/cs50.c They are saying stuff like: string name = get_string("What is your name?"); Then, they never call free on the memory in main. Anyone know how the memory gets freed or if it is a leak? Thanks.
No. From the very code you linked to:
/**
* Called automatically after execution exits main.
*/
static void teardown(void)
{
// Free library's strings
if (strings != NULL)
{
for (size_t i = 0; i < allocations; i++)
{
free(strings[i]);
}
free(strings);
}
}The library keeps track of all the strings it allocates and frees them when your app exits.
Asking questions is a skill CodeProject Forum Guidelines Google: C# How to debug code Seriously, go read these articles. Dave Kreskowiak
-
I am taking Harvard's CS50 class. We are learning C. They use their own library where you include cs50.h. There is a function called get_string that returns a string. I think it has a memory leak. I think this is the source code: https://github.com/cs50/libcs50/blob/main/src/cs50.c They are saying stuff like: string name = get_string("What is your name?"); Then, they never call free on the memory in main. Anyone know how the memory gets freed or if it is a leak? Thanks.
After Dave's on-point answer, this is just a general comment with the hope that your question becomes a teachable moment. Way back when, as I was learning programming, I came up with a saying: "the silly is on the other side of the screen". It obviously can be interpreted both ways, but the idea is to look where the balance of probability indicates that the error should be. In my case, I learned my first assembly language trying to prove that the compiler was wrong - of course it wasn't. In your case, the chance that a course from a prestigious institution contains a trivial error are rather small compared with the chance that you are missing something. Try to find what you are missing and those "Aha!" moments may benefit you more than the rest of the assignment. And, when in doubt, try to remember: the silly is on the other side of the screen. :laugh:
Mircea
-
After Dave's on-point answer, this is just a general comment with the hope that your question becomes a teachable moment. Way back when, as I was learning programming, I came up with a saying: "the silly is on the other side of the screen". It obviously can be interpreted both ways, but the idea is to look where the balance of probability indicates that the error should be. In my case, I learned my first assembly language trying to prove that the compiler was wrong - of course it wasn't. In your case, the chance that a course from a prestigious institution contains a trivial error are rather small compared with the chance that you are missing something. Try to find what you are missing and those "Aha!" moments may benefit you more than the rest of the assignment. And, when in doubt, try to remember: the silly is on the other side of the screen. :laugh:
Mircea
This is their pseudocode for bubble sort: -------------------------------------------------------
Repeat n-1 times
For i from 0 to n–2
If numbers[i] and numbers[i+1] out of order
Swap them
If no swaps
Quit------------------------------------------------------- Then, David Malan supposedly analyzed the running time by multiplying n-1 by n-2.
-
This is their pseudocode for bubble sort: -------------------------------------------------------
Repeat n-1 times
For i from 0 to n–2
If numbers[i] and numbers[i+1] out of order
Swap them
If no swaps
Quit------------------------------------------------------- Then, David Malan supposedly analyzed the running time by multiplying n-1 by n-2.
That is, indeed, the worst case performance of bubble sort. Bubble sort - Wikipedia[^]. Not sure what you want to say.
Mircea
-
This is their pseudocode for bubble sort: -------------------------------------------------------
Repeat n-1 times
For i from 0 to n–2
If numbers[i] and numbers[i+1] out of order
Swap them
If no swaps
Quit------------------------------------------------------- Then, David Malan supposedly analyzed the running time by multiplying n-1 by n-2.
What does that have to do with "get_string memory leak"?
Asking questions is a skill CodeProject Forum Guidelines Google: C# How to debug code Seriously, go read these articles. Dave Kreskowiak
-
I am taking Harvard's CS50 class. We are learning C. They use their own library where you include cs50.h. There is a function called get_string that returns a string. I think it has a memory leak. I think this is the source code: https://github.com/cs50/libcs50/blob/main/src/cs50.c They are saying stuff like: string name = get_string("What is your name?"); Then, they never call free on the memory in main. Anyone know how the memory gets freed or if it is a leak? Thanks.
mike7411 wrote:
I think it has a memory leak.
Despite the other posts - yes this would be a classic example of a memory leak. The teardown() method, as used, is pointless. When the application exits (on every modern OS including small ones) the memory that the application uses is returned to the OS. So the fact that teardown is called on the application exit via the atexit() method is absolutely pointless. If you were to use the code in a regular application, especially but not limited to one that does not exit, then this would fail. Because of memory leaks. You could not call teardown() arbitrarily either because it would clear everything. Now it is possible that the point of the code is not to teach you efficient memory usage. But one could certainly argue that the get_string() method should make it clear that this is not a good way to do things. However the following is in comment for that method.
Stores string
on heap, but library's destructor frees memory on program's exit.And that indicates that the creator of this code did not understand/know what I pointed out above. The code also has a copyright date of 2023 so there is no claim that the code could have been written, for example 50 years ago, where one might (perhaps) have used an OS where the application would have needed to do that. Note however that even then I am not sure any OSes actually worked that way. Back then PC-DOS, CPM and early unix versions (there was no linux) returned memory to the OS. You can take from this that you should not write code like this. In general in C and C++ you should always control the scope of your memory allocations. So for example if your method allocates some memory then that same method should deallocate (all exit conditions for the method included.) There should almost never be a need to have a method return an allocation and if it does happen then the method should document (comment) that the caller is responsible for the allocation. C++ makes this easier since you can wrap the allocation in a class. And control allocation/deallocation from that. That is something that I would suggest always doing. So in C++ you would never return an allocated pointer.
-
mike7411 wrote:
I think it has a memory leak.
Despite the other posts - yes this would be a classic example of a memory leak. The teardown() method, as used, is pointless. When the application exits (on every modern OS including small ones) the memory that the application uses is returned to the OS. So the fact that teardown is called on the application exit via the atexit() method is absolutely pointless. If you were to use the code in a regular application, especially but not limited to one that does not exit, then this would fail. Because of memory leaks. You could not call teardown() arbitrarily either because it would clear everything. Now it is possible that the point of the code is not to teach you efficient memory usage. But one could certainly argue that the get_string() method should make it clear that this is not a good way to do things. However the following is in comment for that method.
Stores string
on heap, but library's destructor frees memory on program's exit.And that indicates that the creator of this code did not understand/know what I pointed out above. The code also has a copyright date of 2023 so there is no claim that the code could have been written, for example 50 years ago, where one might (perhaps) have used an OS where the application would have needed to do that. Note however that even then I am not sure any OSes actually worked that way. Back then PC-DOS, CPM and early unix versions (there was no linux) returned memory to the OS. You can take from this that you should not write code like this. In general in C and C++ you should always control the scope of your memory allocations. So for example if your method allocates some memory then that same method should deallocate (all exit conditions for the method included.) There should almost never be a need to have a method return an allocation and if it does happen then the method should document (comment) that the caller is responsible for the allocation. C++ makes this easier since you can wrap the allocation in a class. And control allocation/deallocation from that. That is something that I would suggest always doing. So in C++ you would never return an allocated pointer.
You've covered everything I was going to say, and then some. I was not sure what early versions of DOS might have done with allocated memory at program exit, but it's nice to know that even then, an most OS's would do the Right Thing™. The code looks like it comes from the early 80's. That it should come from a prestigious university (it is that Harvard, right?), that should know better, is disheartening. Although, it might be a shining example of what not to do.
"A little song, a little dance, a little seltzer down your pants" Chuckles the clown
-
What does that have to do with "get_string memory leak"?
Asking questions is a skill CodeProject Forum Guidelines Google: C# How to debug code Seriously, go read these articles. Dave Kreskowiak
Just the fact that’s not wise to assume the error is someone else’s code. Better to try to understand what’s going on and discover what atexit function does. Next step would be to eventually figure out that the “constructor” code is contrived and the call to atexit could be done on first allocation. Edit: not sure to what message was your reply. Stuck to a cell phone today and CP is not the greatest on mobile. Apologies if I’m confusing you.
Mircea
-
You've covered everything I was going to say, and then some. I was not sure what early versions of DOS might have done with allocated memory at program exit, but it's nice to know that even then, an most OS's would do the Right Thing™. The code looks like it comes from the early 80's. That it should come from a prestigious university (it is that Harvard, right?), that should know better, is disheartening. Although, it might be a shining example of what not to do.
"A little song, a little dance, a little seltzer down your pants" Chuckles the clown
Actually, you, like I do, should question the exisitential "being" of said OP, if this is any indication of a sentience which can't be gleened from his reputation due to the fact that there is no access to said: mike7411 - Professional Profile[^] I know, I know. A kite joke. :(
-
Just the fact that’s not wise to assume the error is someone else’s code. Better to try to understand what’s going on and discover what atexit function does. Next step would be to eventually figure out that the “constructor” code is contrived and the call to atexit could be done on first allocation. Edit: not sure to what message was your reply. Stuck to a cell phone today and CP is not the greatest on mobile. Apologies if I’m confusing you.
Mircea
First the question is about a method that may leak memory, then a sudden jump-cut a post about bubble sort pseudocode, then another jump-cut to your philosophy. To say I'm confused as to what's going on with this thread is an understatement.
Asking questions is a skill CodeProject Forum Guidelines Google: C# How to debug code Seriously, go read these articles. Dave Kreskowiak
-
First the question is about a method that may leak memory, then a sudden jump-cut a post about bubble sort pseudocode, then another jump-cut to your philosophy. To say I'm confused as to what's going on with this thread is an understatement.
Asking questions is a skill CodeProject Forum Guidelines Google: C# How to debug code Seriously, go read these articles. Dave Kreskowiak
Apologies again! I thought your message was directed at my answer. Anyway I'll try to cut the philosophy c**p in the future; it's waste of time.
Mircea
-
You've covered everything I was going to say, and then some. I was not sure what early versions of DOS might have done with allocated memory at program exit, but it's nice to know that even then, an most OS's would do the Right Thing™. The code looks like it comes from the early 80's. That it should come from a prestigious university (it is that Harvard, right?), that should know better, is disheartening. Although, it might be a shining example of what not to do.
"A little song, a little dance, a little seltzer down your pants" Chuckles the clown
k5054 wrote:
That it should come from a prestigious university (it is that Harvard, right?), that should know better
That of course doesn't mean anything. Large organizations, all large organizations, tend towards the average. That is inevitable due to size. Besides that there is no determination about an individuals competence in general or for specific things. You can also google for variations of the code library and find variations. I found a variation that looked like it was C++. No memory leak in that (very brief look) because it was returning 'string'. So possible that someone familiar with C++ but not so much C converted it. And also possible it wasn't even the person teaching the class, just that the teacher found it and is using it.
-
k5054 wrote:
That it should come from a prestigious university (it is that Harvard, right?), that should know better
That of course doesn't mean anything. Large organizations, all large organizations, tend towards the average. That is inevitable due to size. Besides that there is no determination about an individuals competence in general or for specific things. You can also google for variations of the code library and find variations. I found a variation that looked like it was C++. No memory leak in that (very brief look) because it was returning 'string'. So possible that someone familiar with C++ but not so much C converted it. And also possible it wasn't even the person teaching the class, just that the teacher found it and is using it.