Sorry, this is probably a really basic question, but can I free() buffer if it was allocated like char buffer[2047];? I understand if a variable is out of scope it is leaked?
–
ryystMar 27 '10 at 14:38

@ryyst: If it's allocated as an array like you originally did, it can't get leaked. But if it's dynamically allocated, and the pointer goes out of scope without you freeing first, then it'll be leaked. That's why you generally avoid dynamic allocation if possible - it leaves you open to leaks if you're not careful.
–
JefromiMar 27 '10 at 14:40

@ryyst No, the compiler allocated the buffer in the first example (on the stack) and the compiler will take care of deallocating it.
–
anonMar 27 '10 at 14:41

2

@rryst you seem to be pretty confused about how memory management and the stack work. You might want to read up on that a bit.
–
SoapBoxMar 27 '10 at 14:41

It's a good idea to make the buffer at least one byte longer than the number of characters you expect to read so that you can hold the terminating NUL too. Aside from that, the original won't leak; once the function containing the code returns, those variables are gone. If there is a leak with it, it must be in something you're not showing us.

So the memory for the buffer in my first version gets deallocated when it gets out of scope? I always thought it would become a "dangling" pointer that just takes up space...
–
ryystMar 27 '10 at 14:41

+1 for being the only one so far to have noticed the buffer overflow bug as well.
–
Dan MouldingMar 27 '10 at 14:58

To answer "what am I doing wrong", you are dereferencing your buffer pointer. Since it is declared as char *, when you write *buffer you're getting the first character in the buffer, you want the whole thing so just remove the * from the front, like this:

if(fscanf(file, "%2047[^\n]%n%*c", buffer, charsRead) == 1) {

But, you seem to have a faulty premise. Using a statically allocated (on the stack) array as in your first code snippet does not "leak memory." Quite the opposite, since you don't have any frees in your second code snippet, that is the code that will leak memory. If you know at compile time the size that the array will need to be (in this case, 2047), then you should (usually, there are always exceptions but you're not that advanced yet) use a static array instead of dynamically allocating one.

I see three problems with your code. First, to answer the question you posed, you should pass buffer rather than *buffer as the third argument to fscanf. Neil's answer does a good job explaining why.

Second, you have a buffer overflow. fscanf automatically appends a null terminating character to the scanned input. The buffer you give to fscanf must have enough space for the scanned input and the null terminating character. If you want to scan 2,047 characters then your buffer needs to be 2,048 characters long.

Third, the new version of your code is the one with the memory leak. Your previous version had no leaks because the buffer there was allocated on the stack (or in static storage if it is a global variable). The stack space used by the buffer will be reclaimed when the function returns. Allocating the buffer from the heap by using malloc means that you are responsible for reclaiming the allocated heap memory by subsequently calling free when you are finished with the buffer. In my opinion, your original version of the code, where you allocated buffer on the stack, was much better.

The only case where the new version might be preferable is if you are targeting a system that has very limited stack space (as is the case with some embedded systems). On such systems, allocating a large buffer on the stack might not be a good idea. In such a case it may be preferable to allocate the buffer from the heap using malloc, to avoid possible stack overflows. If that's the case then you must be careful to avoid memory leaks by calling free to deallocate the memory.