patterncppMinor
Mimic sprintf with std::string output
Viewed 0 times
mimicstdwithoutputsprintfstring
Problem
I know that stringstreams are the C++ recommended way to create formatted text. However, they can often become quite verbose, especially when compared to the succinct format strings of
My goal was to make a function to behave similarly to
printf and family. However, the printf family can lead to its own issues, with buffer overflow issues, and so I would rather not use these functions directlyMy goal was to make a function to behave similarly to
snprintf, but return a std::string, avoiding any chance of buffer overflow. Knowing that there are many pitfalls to string manipulation in C++, have I exposed myself to any errors in using this function?#include
#include
#include
std::string string_sprintf(const std::string& format, ...){
static const int initial_buf_size = 100;
va_list arglist;
va_start(arglist, format);
char buf1[initial_buf_size];
const int len = vsnprintf(buf1,initial_buf_size,format.c_str(), arglist) + 1;
va_end(arglist);
if(len<initial_buf_size){
return buf1;
} else {
char buf2[len];
va_start(arglist,format);
vsnprintf(buf2,len,format.c_str(),arglist);
va_end(arglist);
return buf2;
}
}Solution
Since you're using
It's literally impossible, since there's no way to forward the
Using
Note that using a
Oh, and of course you could do a hybrid where you use a variadic template, but only to forward the arguments to
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
Think of it this way:
I wouldn't bother using
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:
and
Your function has way too much duplicated code. Let the
Basically what I'm saying is that just as the standard library has
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 (
Instead, I would use a container like a
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_e
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_e
Code Snippets
func(param1, param2, param3)if (a < b) {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;
}Context
StackExchange Code Review Q#52522, answer score: 7
Revisions (0)
No revisions yet.