1

I'm trying to write a few wrappers for the standard sprintf function from cstdio. However, I'm having some weird behaviour and access violation crashes when running my program. I've simplified the problem and reproduced it on the code below:

#include <string>
#include <cstdio>
#include <cstdarg>

std::string vfmt(const std::string& format, va_list args)
{
    int size = format.size() * 2;
    char* buffer = new char[size];
    while (vsprintf(buffer, format.c_str(), args) < 0)
    {
       delete[] buffer;
       size *= 2;
       buffer = new char[size];
    }

    return std::string(buffer);
}

std::string fmt(const std::string& format, ...)
{
    va_list args;
    va_start(args, format);
    std::string res = vfmt(format, args);
    va_end(args);
    return res;
}

int main()
{
    std::string s = fmt("Hello %s!", "world");
    printf(s.c_str());
    return 0;
}

This code produces a memory access violation when calling vsprintf in vfmt. However, when I change fmt's function signature from fmt(const std::string& format, ...) to fmt(const char* format, ...), I no longer crash, and everything works as expected. Why exactly is this happening?

Why is it that changing the type of the format parameter from const std::string& to const char* solves the issue? Or does it only appear to be solved?

Zeenobit
  • 4,954
  • 3
  • 34
  • 46
  • 1
    @dirkgently No, that while loop is not for parsing the `args`. It's calling `vsprintf` and checking if the result string fits the buffer. If not, it resizes the buffer and tries again until it does. – Zeenobit Jun 18 '12 at 18:39
  • 1
    Is there any reason you're using the unsafe `vsprintf` instead of `vsnprintf`, or any of its C++ replacements? – Fred Foo Jun 18 '12 at 18:40
  • Ah yes, I realized that about 5 mins after posting. My bad. :/ – dirkgently Jun 18 '12 at 18:55
  • there is bypass for this error : `#define _CRT_NO_VA_START_VALIDATION` some people have seen this error starting to compile with VS2015, previously no problem. – Sandburg Apr 24 '18 at 12:54

2 Answers2

3

You can't use a reference type as an argument to va_start. That's why changing to char* works, and so would leaving the string but without the &. Using a reference messes up the magic that is done in order to obtain the variable number of arguments.

See Are there gotchas using varargs with reference parameters.

Community
  • 1
  • 1
Eran
  • 21,632
  • 6
  • 56
  • 89
1

You can't do that. I mean, the version you say "works".

vsprintf doesn't let you detect when the buffer passed in is too small, since it doesn't know how big it is. It will happily write over whatever objects follow your buffer. This may cause an access violation, it may crash your program sometime later, it may generate an e-mail to your mother with racy pictures attached. This is undefined behavior. Your reallocate-and-retry loop is useless.

You may have better luck with vsnprintf, which does know how big the buffer is, if your library provides it.

Ben Voigt
  • 277,958
  • 43
  • 419
  • 720