Using fscanf with dynamically allocated buffer.

Question

Hi,

I got the following code:

char buffer[2047];
int charsRead;

do {
    if(fscanf(file, "%2047[^\n]%n%*c", buffer, &charsRead) == 1) {
        // Do something
    }
} while (charsRead == 2047);

I wanted to convert this code to use dynamically allocated variables so that when calling this code often I won't get heavy memory leakage. Thus, I tried this:

char *buffer = malloc(sizeof(char) * 2047);
int *charsRead = malloc(sizeof(int));

do {
    if(fscanf(file, "%2047[^\n]%n%*c", *buffer, charsRead) == 1) {
        // Do something
    }
} while (*charsRead == 2047);

Unfortunately, this does not work. I always get “EXC_BAD_ACCESS” errors, just before the if-statement with the fscanf call. What am I doing wrong?

Thanks for any help!

-- Ry

Answer 1

The original code is far less likely to leak than the new, as the compiler is managing the memory for you, but if you feel must, change to:

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

You don't want to dereference buffer here, any more than you did in the first bit of code. Doing so would give you the first character in the buffer, but you want the buffer's address.

Answer 2

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.

Answer 3

You don't accomplish anything by making charsRead dynamically allocated. Go back to how you had charsRead before and perhaps that will solve your problem.

Answer 4

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.

Answer 5

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.