16

I am using sprintf function in C++ 11, in the following way:

std::string toString()
{
    std::string output;
    uint32_t strSize=512;
    do
    {
        output.reserve(strSize);
        int ret = sprintf(output.c_str(), "Type=%u Version=%u ContentType=%u contentFormatVersion=%u magic=%04x Seg=%u",
            INDEX_RECORD_TYPE_SERIALIZATION_HEADER,
            FORAMT_VERSION,
            contentType,
            contentFormatVersion,
            magic,
            segmentId);

        strSize *= 2;
    } while (ret < 0);

    return output;
}

Is there a better way to do this, than to check every time if the reserved space was enough? For future possibility of adding more things.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Dan The Man
  • 1,835
  • 6
  • 30
  • 50
  • 1
    Are you using `snprintf`? Because `sprintf`, as shown in your code, has no way to determine the buffer size. `snprintf` would also return the required buffer size, so you could just use the returned value +1 as new `strSize`. – Karsten Koop Apr 28 '16 at 08:20
  • 1
    This code is very wrong. `reserve` does not change the size of the string, and `sprintf` does not return negative just because you wrote out of bounds . You need to allocate the space you need *before* writing out of bounds. – M.M Apr 28 '16 at 08:25
  • Related question: http://stackoverflow.com/questions/2342162/stdstring-formatting-like-sprintf – Thomas Perl Sep 16 '16 at 09:44

7 Answers7

22

Your construct -- writing into the buffer received from c_str() -- is undefined behaviour, even if you checked the string's capacity beforehand. (The return value is a pointer to const char, and the function itself marked const, for a reason.)

Don't mix C and C++, especially not for writing into internal object representation. (That is breaking very basic OOP.) Use C++, for type safety and not running into conversion specifier / parameter mismatches, if for nothing else.

std::ostringstream s;
s << "Type=" << INDEX_RECORD_TYPE_SERIALIZATION_HEADER
  << " Version=" << FORMAT_VERSION
  // ...and so on...
  ;
std::string output = s.str();

Alternative:

std::string output = "Type=" + std::to_string( INDEX_RECORD_TYPE_SERIALIZATION_HEADER )
                   + " Version=" + std::to_string( FORMAT_VERSION )
                   // ...and so on...
                   ;
DevSolar
  • 67,862
  • 21
  • 134
  • 209
  • 3
    how can i simulate the string formatting in ostringstream, like magic=%04x and so on? – Dan The Man Apr 28 '16 at 08:48
  • 1
    @danieltheman: [`setw`](http://en.cppreference.com/w/cpp/io/manip/setw), [`setfill`](http://en.cppreference.com/w/cpp/io/manip/setfill). – DevSolar Apr 28 '16 at 08:50
  • Your comment *'The return value is a pointer to **const** char, and the function itself marked **const**, for a reason'* implies that the code presented *should not even compile* due to `const` qualifier mismatch at `sprintf(output.c_str(), ...` expression. So it's false OP *is using* `sprintf` the way he described, because a presented excerpt is not a runnable code... – CiaPan Apr 28 '16 at 09:57
  • 2
    @CiaPan it doesn't technically imply that the code should not even compile. If we assume the knowledge that implicit conversion from const to non-const is ill-formed, then it implies that a compiler is not *required* to compile the code, but the compiler can support the conversion as a non-standard language extension. The compiler is simply required to give the user a diagnostic message, if the program is ill-formed according to the standard. *If* it does compile, then the write through the const pointer is UB. – eerorika Apr 28 '16 at 10:48
  • @user2079303 Got it. Thank you for the explanation, I thought compilers are not allowed to accept such code. – CiaPan Apr 28 '16 at 11:17
  • @CiaPan: Quite generally speaking, a compiler's warnings and a language's rules are only losely coupled. This is doubly true for C/C++, which made it a credo that "the developer knows what he's doing". – DevSolar Apr 28 '16 at 11:41
16

The C++ patterns shown in other answers are nicer, but for completeness, here is a correct way with sprintf:

auto format = "your %x format %d string %s";
auto size = std::snprintf(nullptr, 0, format /* Arguments go here*/);
std::string output(size + 1, '\0');
std::sprintf(&output[0], format, /* Arguments go here*/);

Pay attention to

  • You must resize your string. reserve does not change the size of the buffer. In my example, I construct correctly sized string directly.
  • c_str() returns a const char*. You may not pass it to sprintf.
  • std::string buffer was not guaranteed to be contiguous prior to C++11 and this relies on that guarantee. If you need to support exotic pre-C++11 conforming platforms that use rope implementation for std::string, then you're probably better off sprinting into std::vector<char> first and then copying the vector to the string.
  • This only works if the arguments are not modified between the size calculation and formatting; use either local copies of variables or thread synchronisation primitives for multi-threaded code.
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
eerorika
  • 232,697
  • 12
  • 197
  • 326
  • 1
    Interesting; I am not quite sure if the construct (writing to `&output[0]`) is adhering to the letter of the standard, though. This will probably work for next to all implementations of `std::string`, but as for well-definedness, I have doubts. – DevSolar Apr 28 '16 at 08:59
  • @DevSolar If I remember correctly, contiguousness of `std::string` buffer is guaranteed since c++11. Prior to that, it was not guaranteed. As far as I know, there are no other reasons for this to not work. – eerorika Apr 28 '16 at 09:04
  • @user2079303 string constructor that gets a number in it? i don`t think there is such thing in c++, can you please elaborate? i am talking about this: std::string output(size + 1); – Dan The Man Apr 28 '16 at 10:17
  • @danieltheman sorry, I meant to use the constructor that takes a size, and a `char`. I forgot that the second argument has no default value. The code is now fixed. – eerorika Apr 28 '16 at 10:20
  • Wouldn't you want to use `std::snprintf(&output[0], size, format, /* args go here */);` instead of the last line to be safer? Imagine that another thread modifies one of the format string args (say, an integer goes from 9 to 10, thereby taking up one more character) between the `auto size = ...` assignment and the `std::sprintf()` call at the end. – Thomas Perl Sep 16 '16 at 09:41
  • @ThomasPerl The size calculation, buffer allocation, and the output operation really only make sense in a non-data-race context. That's a good point. The size accepting version of `sprintf` would indeed avoid UB in a data race, but you would still have the possibility of arbitrarily cut-off output. You really should use a critical section to avoid a race here - and if you do, then the non-size and size using versions are equally safe. Or alternatively, simply use local copies of the non-const arguments so that they cannot be modified by other threads. – eerorika Sep 16 '16 at 09:57
  • Many code analysis tools will warn about sprintf(), so makes sense to always use snprintf(). – jpa Oct 14 '16 at 07:28
  • It's good. The string should be resized again at the end of the code snippet to snip off the terminating null. – Buster Oct 22 '19 at 12:16
  • Why not `std::string output(size, '\0');`, without the +1? That [constructor (4)](https://en.cppreference.com/w/cpp/string/basic_string/basic_string) copies N times the character AND adds a terminating `\0`, effectively having `size+1` bytes. If you pass in `size+1`, it ends up having `size+2` bytes, with the last "real" char set to `\0`, which will cause problems when concatenating, size computing and so – LoPiTaL Nov 30 '21 at 07:56
6

We can mix code from here https://stackoverflow.com/a/36909699/2667451 and here https://stackoverflow.com/a/7257307 and result will be like that:

template <typename ...Args>
std::string stringWithFormat(const std::string& format, Args && ...args)
{
    auto size = std::snprintf(nullptr, 0, format.c_str(), std::forward<Args>(args)...);
    std::string output(size + 1, '\0');
    std::sprintf(&output[0], format.c_str(), std::forward<Args>(args)...);
    return output;
}
Community
  • 1
  • 1
zergeny
  • 59
  • 1
  • 4
  • I don't think you should add 1 to the string size for the null terminator. `std::string` doesn't require it. It should be rather `std::string output(size, '\0');` instead. – vahancho Jan 18 '23 at 14:09
2

A better way is to use the {fmt} library. Ex:

std::string message = fmt::sprintf("The answer is %d", 42);

It exposes also a nicer interface than iostreams and printf. Ex:

std::string message = fmt::format("The answer is {}", 42);`

See:
https://github.com/fmtlib/fmt
http://fmtlib.net/latest/api.html#printf-formatting-functions

vitaut
  • 49,672
  • 25
  • 199
  • 336
1

Your code is wrong. reserve allocates memory for the string, but does not change its size. Writing into the buffer returned by c_str does not change its size either. So the string still believes its size is 0, and you've just written something into the unused space in the string's buffer. (Probably. Technically, the code has Undefined Behaviour, because writing into c_str is undefined, so anything could happen).

What you really want to do is forget sprintf and similar C-style functions, and use the C++ way of string formatting—string streams:

std::ostringstream ss;
ss << "Type=" << INDEX_RECORD_TYPE_SERIALIZATION_HEADER
   << " Version=" << FORAMT_VERSION
   << /* ... the rest ... */;
return ss.str();
Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
0

Yes, there is!

In C, the better way is to associate a file with the null device and make a dummy printf of the desired output to it, to learn how much space would it take if actually printed. Then allocate appropriate buffer and sprintf the same data to it.

In C++ you could associate the output stream with a null device, too, and test the number of charactes printed with std::ostream::tellp. However, using ostringstream is a way better solution – see the answers by DevSolar or Angew.

CiaPan
  • 9,381
  • 2
  • 21
  • 35
  • 1
    Completely missing the point that you cannot write to the buffer returned by `c_str()`, and needlessly complicated even then since there is `snprintf()` / `snprintf_s`. – DevSolar Apr 28 '16 at 08:38
  • `snprintf` is better than the null device hack. But you have to use that hack for `swprintf`. – M.M Apr 28 '16 at 08:57
0

You can use an implementation of sprintf() into a std::string I wrote that uses vsnprintf() under the hood.

It splits the format string into sections of plain text which are just copied to the destination std::string and sections of format fields (such as %5.2lf) which are first vsnprintf()ed into a buffer and then appended to the destination.

https://gitlab.com/eltomito/bodacious-sprintf

eltomito
  • 280
  • 1
  • 8