strprintf - is this implementation safe and portable?

Question

I'm thinking about proposing the following as an alternative to sprintf/snprintf in our project.

The motivation is to remove the need to think about buffer sizes and keep as much of the convenience of the original as possible.

std::string strprintf(const char *format, ...)
{
    std::string s;
    s.resize(128); // best guess
    char *buff = const_cast<char *>(s.data());

    va_list arglist;
    va_start(arglist, format);
    auto len = vsnprintf(buff, 128, format, arglist);
    va_end(arglist);

    if (len > 127)
    {
        va_start(arglist, format);
        s.resize(len + 1); // leave room for null terminator
        buff = const_cast<char *>(s.data());
        len = vsnprintf(buff, len+1, format, arglist);
        va_end(arglist);
    }
    s.resize(len);
    return s; // move semantics FTW
}

Does this code have any inherent problems?

usage example:

auto s = strprintf("Hello %d world", 777);

It seems technically correct, but not very performant if the buffer exceeds your 128 size cap. It calls vsnprintf again, making the function take twice as long. You have to weigh between convenience and performance here. Additionally, the string you return will be copied again - which further impacts performance. — Freakyy, Aug 10 '17 at 08:22
The return value has move semantics, so there shouldn't be another copy. — CplusPuzzle, Aug 10 '17 at 08:53
I think the compiler only optimizes `vsnprintf` with a size of 0. Also, `&s[0]` to avoid the `const_cast`. — o11c, Aug 14 '17 at 05:25
casting away const on `s.data()` and writing to it is undefined behaviour. You should use `&s[0]` — M.M, Aug 14 '17 at 05:33
A major problem with this is that it breaks for incorrect format specifier still — M.M, Aug 14 '17 at 05:34
So, all the type safety of sprintf combined with all the extensibility of sprintf? Anyway, [codereview.se] may be a better fit — n. m. could be an AI, Aug 14 '17 at 05:45

score 3 · Answer 1 · answered Mar 01 '23 at 13:44

The currently suggested implementation seems to have at least the following problems with regard to safety and portability:

Don't use const_cast. If you're using C++17, then you can just leave out the const_cast, as in C++17, data() returns a char* for non-const strings. In C++11 and C++14, buf=&s[0] can be used as an alternative. Pre-C++11 you would have to allocate a char[] array with new[] and delete it with delete[], as there was no guarantee that the characters of a std::string would be stored sequentially.
vsnprintf can return negative values on errors, e.g. format string errors. Depending on what negative value is returned (implementation-defined), the resize(len+1) call will resize the string either to new length 0 and hide errors, or to a very large new length (e.g. size_t(-1)), which may trigger an out-of-memory condition. You may want to add error handling to your function to avoid both.
If the first vsnprintf returns len==INT_MAX, then computing (len+1) has undefined behavior. Avoid by checking.
Behavior and return values of vsnprintf functions may be different from those defined in C99 for Microsoft compilers and for some pre-C++11 C++ compilers. When portability to those is a requirement, be sure to check docs and perform tests on border cases.
Regarding safety: Modern compilers can check the data types of parameters to printf-type functions against the format string, and warn if they detect usages where the data type of a parameter does not match the format string. By providing your own printf-type function, you lose this compiler check at first. Some compilers support adding __attribute__s to your function so that printf-format checking can be enabled for your custom function. You should enable this for all compilers that you target and that support this feature.

Great notes. Do you happen to know if there was ever an implementation where the std::string storage was not contiguous? — CplusPuzzle, Mar 02 '23 at 16:13

score 0 · Accepted Answer · answered Aug 14 '17 at 05:22

0

Turns out that there's already much discussion on this here:

std::string formatting like sprintf

Subjectively, my version still looks more concise than most stuff there, but it's functionally identical to some of the proposed solution on that thread.

answered Aug 14 '17 at 05:22

CplusPuzzle

298
2
9

strprintf - is this implementation safe and portable?

2 Answers2