views:

90

answers:

1

How would I only allocate as much memory as really needed without knowing how big the arguments to the function are?

Usually, I would use a fixed size, and calculate the rest with sizeof (note: the code isn't supposed to make sense, but to show the problem):

#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>

int test(const char* format, ...)
{
    char* buffer;
    int bufsize;
    int status;
    va_list arguments;
    va_start(arguments, format);
    bufsize = 1024; /* fixed size */
    bufsize = sizeof(arguments) + sizeof(format) + 1024;
    buffer = (char*)malloc(bufsize);
    status = vsprintf(buffer, format, arguments);
    fputs(buffer, stdout);
    va_end(arguments);
    return status;
}

int main()
{
    const char* name = "World";
    test("Hello, %s\n", name);
    return 0;
}

However, I don't think this is the way to go... so, how would I calculate the required buffersize properly here?

+7  A: 

If you have vsnprintf available to you, I would make use of that. It prevents buffer overflow since you provide the buffer size, and it returns the actual size needed.

So allocate your 1K buffer then attempt to use vsnprintf to write into that buffer, limiting the size. If the size returned was less than or equal to your buffer size, then it's worked and you can just use the buffer.

If the size returned was greater than the buffer size, then call realloc to get a bigger buffer and try it again. Provided the data hasn't changed (e.g., threading issues), the second one will work fine since you already know how big it will be.

This is relatively efficient provided you choose your default buffer size carefully. If the vast majority of your outputs are within that limit, very few reallocations has to take place (see below for a possible optimisation).

If you don't have an vsnprintf-type function, a trick we've used before is to open a file handle to /dev/null and use that for the same purpose (checking the size before outputting to a buffer). Use vfprintf to that file handle to get the size (the output goes to the bit bucket), then allocate enough space based on the return value, and vsprintf to that buffer. Again, it should be large enough since you've figured out the needed size.


An optimisation to the methods above would be to use a local buffer, rather than an allocated buffer, for the 1K chunk. This avoids having to use malloc in those situations where it's unnecessary, assuming your stack can handle it.

In other words, use something like:

int test(const char* format, ...)
{
    char buff1k[1024];
    char *buffer = buff1k; // default to local buffer, no malloc.
    :
    int need = 1 + vsnprintf (buffer, sizeof (buff1k), format, arguments);
    if (need > sizeof (buff1k)) {
        buffer = malloc (need);
        // Now you have a big-enough buffer, vsprintf into there.
    }

    // Use string at buffer for whatever you want.
    ...

    // Only free buffer if it was allocated.
    if (buffer != buff1k)
        free (buffer);
}
paxdiablo
nice, and a clever solution too. I like it. Thank you :-)
nebukadnezzar
I've modified the code, so it'll accept a constant bufsize for `malloc`: http://codepad.org/qiYM0ZHf (a check with valgrind also showed no memleaks)
nebukadnezzar
@nebukadnezzar, I see two immediate problems with the code you pasted. 1/ When the need is greater than initial bufsize, neither buffer nor mainbuf will be freed (set initial_bufsize to 20 to check this - valgrind will probably only catch the leak if it actually occurs, which it doesn't in your sample). 2/ Your second vsnprintf should use need as the size rather that initial_bufsize. There's no point allocating 1.5K if you're still going to limit the output to 1K.
paxdiablo
You can fix the second by simply changing the size argument. To fix the first, always free mainbuf and only free buffer if it's not equal to mainbuf. The other thing to be wary of, I'm not sure if you can do two v*printf calls without an intervening va_end/va_start sequence. I have a vague recollection that that was undefined behaviour but I'm not sure. If it's working, that may be by accident.
paxdiablo
If there's a reason why you don't want to use 1K on the stack (environment limitations for example), that's okay, but I think it would simplify (and speed up) things if you considered doing it that way. The solution you have will work of course but it seems slightly wasteful, tho' that's just my opinion, feel free to ignore the ravings on an old codger :-)
paxdiablo
there is no way I would ignore the recommendations of an experienced programmer :-) I did indeed follow your advice, and modified the code; and of course I'm always open for good advices!
nebukadnezzar
Don't use `realloc` if the buffer is too small; simply allocate a new one. Using `realloc` will unnecessarily preserve the partial output, likely copying it to a new location, when it's just going to be overwritten anyway.
R..
@R, "likely" depends on the `malloc` engine and the current state of the arena but you're probably right. It's at least as valid to do a `free/malloc` pair in lieu of `realloc`, since you're going to overwrite it anyway, and it may well be faster or more efficient. I still prefer the stack-based buffer for the initial attempt myself, followed by a `malloc` if you need extra space, since that does no heap manipulation for the most part.
paxdiablo