4
\$\begingroup\$

I'm experimenting with writing a dynamic string library in C, and I decided to write an implementation of sprintf.

In my code, I call vsnprintf with a buffer of 0 bytes, just so I can get the number of bytes it requires. I was wondering if there is anything wrong with this code and if there a better way to do this. It feels wrong to me, but I can't seem to think of a better way to do so yet have the same effect.

string string_printf(const char *fmt, ...) {
    va_list in, copy;
    string a = NULL;
    int size;

    /* start the main input */
    va_start(in, fmt);
    /* make a copy of the input */
    va_copy(copy, in);

    /* run vsnprintf just to see how much bytes needed */
    size = vsnprintf(a, 0, fmt, copy) + 1;
    va_end(copy);

    /* allocate the amount now */
    a = string_new_size(size);
    string_to_sr(a)->len = size - 1;

    /* re-run */
    vsnprintf(a, size, fmt, in);
    va_end(in);

    /* done */
    return a;
}
\$\endgroup\$
1
\$\begingroup\$

Looks reasonable to me.

The efficiency of this code really depends on your expected workload. For example, if you're expecting to use this code with a lot of strings of length 50 to 60, and you don't mind wasting a bit of memory in the case that the strings are shorter, then you could probably avoid a vsnprintf in the common case by writing

string string_printf(const char *fmt, ...) {
    va_list in, copy;
    const int INITIAL_SIZE = 60;
    string a = string_new_size(INITIAL_SIZE);

    /* start the main input */
    va_start(in, fmt);
    /* make a copy of the input */
    va_copy(copy, in);

    /* run vsnprintf just to see how much bytes needed */
    int length = vsnprintf(a, INITIAL_SIZE, fmt, copy);
    va_end(copy);

    /* allocate the amount now */
    if (length >= INITIAL_SIZE) {
        /* re-run */
        a = string_new_size(length+1);
        int new_length = vsnprintf(a, length+1, fmt, in);
        assert(new_length <= length);
    }
    string_to_sr(a)->len = length;
    va_end(in);

    /* done */
    return a;
}

Notice that I've also changed your size = ... + 1, size-1 to length = ..., length, on the principle that adding and subtracting 1 all the time is less efficient than not adding and subtracting 1 all the time; and on the principle that everybody knows what "string length" means (it's strlen), whereas "string size" is a bit ambiguous (does it include the null terminator or not?).


Somewhat off-topic: I'm leery of your use of string a; to declare a pointer. To me, a bare identifier always means "value-semantic object", as in FILE or char; and if the object is supposed to have pointer semantics, you add a star, as in FILE* or char*. Hiding the star inside a typedef strikes me as a bad idea. Although I admit that if you're coming from Java or Objective-C or Python or any other non-C, non-C++ language where

a = b;

traditionally means "don't copy the value, just reseat the pointer a to point to the same heap object as b", then your use of string a; a->something(); will seem perfectly natural. To me it seems confusing; I'd rather write string *a; a->something();.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.