Since you're using cstdarg and va_list is a complete type, it should be std::va_list.
va_start with a std::string is undefined behavior. This means that you actually cannot use std::string as your format (at least not if you plan on using only the stdargs facilities).
It's literally impossible, since there's no way to forward the ... idea, but instead you can only forward a va_list, which of course presents a problem since you can't obtain a va_list with a std::string parameter.
Using vsnprintf implies that you're using C++11. Given this, I might be tempted to go the variadic template route rather than leveraing the old C style functions that don't have type safety. Unfortunately though, much of the parsing and transforming logic would have to be recreated. If you do want to consider going this route, Andrei Alexandrescu gave a talk in which he examined this option.
Note that using a std::string format with variadic templates is of course possible.
Oh, and of course you could do a hybrid where you use a variadic template, but only to forward the arguments to vsnprintf. That would allow you to use a std::string format but you don't get the true typesafety of the variadic approach.
vnp already covered this, but let's take a second look and consider an excerpt from the standard:
The vsnprintf function returns the number of characters that would have been written
had n been sufficiently large, not counting the terminating null character, or a negative
value if an encoding error occurred. Thus, the null-terminated output has been
completely written if and only if the returned value is nonnegative and less than n.
Unfortunately, you'll have to do some pretty gross handling for this since on MS systems a negative number currently means the buffer was not large enough whereas on standard compliant systems it means some of encoding error happened.
If you include the null character in the calculation, you need to use <=. If you don't include it, you need to use <. This means that your current logic of len < initial_buf_size is wrong. It should actually be len <= initial_buf_size.
Think of it this way: len represents the length of the entire, null terminated output string (since you added 1). initial_buf_size represents the size of the entire buffer, including the required null terminator. Since both numbers include the null terminator, the case where they are equal means that the entire buffer was utilized but that it does contain the full, null terminated output string.
I wouldn't bother using initialize_buf_size. Instead, I would just hard code it into the buf1 declaration and then use sizeof(buf1). I'm all for avoiding magic numbers and pulling them into constants, but it's really only used once (and you should be using sizeof anyway -- the declaration of buf1 can change).
2 spaces is rather unusual in C++. I would expect either 4 spaces or 1 tab.
I'm not a fan of the lack of white space. Like the previous item, this is just opinion, but I (and I think most people -- though obviously I'm prone to confirmation bias on this) prefer spacing around clauses and operators. In other words:
func(param1, param2, param3)
and
if (a < b) {
Your function has way too much duplicated code. Let the va_list function be handled at the top level, and then have a second function that actually does the work. This is actually exactly what every implementation of the standard library I've ever used does with.
Basically what I'm saying is that just as the standard library has printf and vprintf, you would have string_sprintf and string_vsprintf, and string_sprintf would just call string_vsprintf under the hood.
Instead of having two cases, I would be tempted to just do an empty run to vsnprintf to figure out exactly what size you'll need.
You're using a non-standard compiler extension to use a automatic memory duration array that is actually dynamically sized (buf2).
Instead, I would use a container like a std::vector to handle the buffer, or since you're arleady using C++11, you can use std::string since it's guaranteed to be contiguous.
All in all, I might do something like this:
std::string string_vsprintf(const char* format, std::va_list args) {
va_list tmp_args; //unfortunately you cannot consume a va_list twice
va_copy(tmp_args, args); //so we have to copy it
const int required_len = vsnprintf(nullptr, 0, format, tmp_args) + 1;
va_end(tmp_args);
std::string buf(required_len, '\0');
if (std::vsnprintf(&buf[0], buf.size(), format, args) < 0) {
throw std::runtime_error{"string_vsprintf encoding error"};
}
return buf;
}
std::string string_sprintf(const char* format, ...) __attribute__ ((format (printf, 1, 2)));
std::string string_sprintf(const char* format, ...) {
std::va_list args;
va_start(args, format);
std::string str{string_vsprintf(format, args)};
va_end(args);
return str;
}
Note that this does not handle Windows and MS's non-compliant implementation of vsnprintf.
It would also be a good idea to wrap the __attribute__ stuff in preprocessor checks since __attribute__ is only used in GCC and clang.
Oh, and the string is unnecessarily set to all null characters and only then over written with the data we actually care about. If you super care about performance, that might be an issue, but realistically, it shouldn't matter. Anyway around it would require an extra copy anyway (since you'd have to take whatever buffer you used and copy it into a std::string).
