0

I wrote this code in order to use sprintf-style formatting for std::string.

#include <iostream>
#include <string>
#include <vector>
#include <stdarg.h>

std::string formatstr(const std::string &fmt, ...)
{
    const char *fmt_s(fmt.c_str());
    std::vector<char> buf(256);
    va_list args;
    va_start(args, &fmt);
    auto n = -1;
    while ((n = vsnprintf(&buf[0], buf.size() - 1, fmt_s, args)) == -1)
        buf.resize(2 * buf.size());
    va_end(args);
    std::string text(&buf[0]);
    return text;
}

int main(int argc, char** argv)
{
    std::string s = "Instrument label: %s";
    std::cout << formatstr(s, "Frequency Generator") << "\n";

    return EXIT_SUCCESS;
}

Output:

Instrument label: Frequency Generator

It works fine in 64-bit builds.

But, as soon as I change the configuration to 32-bit, the VS editor adds the little red squiggly error line under va_start and complains: expression must be an lvalue or xvalue

VS message

Is it an error that my 64-bit build works? Or an error in Visual Studio that it doesn't for 32-bit? Is there some fundamental difference?

Blair Fonville
  • 908
  • 1
  • 11
  • 23
  • Is it just an intellisense error or do you actually get a compiler error? What if you close and reopen the project with 32-bit already selected? – Retired Ninja May 11 '18 at 22:10
  • @RetiredNinja It's not just intellisense. It actually won't let me compile it in 32-bit. I just tried closing and reopening, but it didn't help. Also, the project is new and minimal - I just made it to test this code (for this question). – Blair Fonville May 11 '18 at 22:12
  • @nvd Ok, I concede that it can be construed as a duplicate. It is odd though, that it works fine in 64-bit builds. I've actually been using it for a while, and only just now tried it in a 32-bit build. – Blair Fonville May 11 '18 at 22:25
  • What's wrong with using C++ style `operator<<` or a variadic template? That would be much safer than trying to use printf style. – Remy Lebeau May 11 '18 at 22:42
  • You can't call vsnprintf twice with the same va_list object, because "As these functions invoke the va_arg macro, the value of ap after the return is unspecified." Maybe Windows lets you get away with that, but the code won't be portable. Also, since VS2015, `vsnprintf` conforms to C99; it no longer returns -1 if the buffer is too short. See https://msdn.microsoft.com/en-us/library/1kt27hek.aspx#Return%20Value – rici May 11 '18 at 23:24
  • @rici Glad you pointed that out. I wrote it when I was using VS2012, and you're right, my resizing loop indeed no longer appears to work. Thanks again. – Blair Fonville May 11 '18 at 23:30

1 Answers1

2

Try replacing va_start(args, &fmt) with va_start(args, fmt)

The documentation (http://www.cplusplus.com/reference/cstdarg/va_start/) does not indicate that a pointer to the argument is required, instead this should be name of the argument preceding the variable arguments: ...

Edit: And also see varargs(va_list va_start) doesn't work with pass-by-reference parameter

So in order to fix it, you could use a pointer instead of a reference for fmt, which give this "fix" (that you may not like it...)

#include <iostream>
#include <string>
#include <vector>
#include <stdarg.h>

std::string formatstr(const std::string *fmt, ...)
{
    const char *fmt_s(fmt->c_str());
    std::vector<char> buf(256);
    va_list args;
    va_start(args, fmt);
    auto n = -1;
    while ((n = vsnprintf(&buf[0], buf.size() - 1, fmt_s, args)) == -1)
        buf.resize(2 * buf.size());
    va_end(args);
    std::string text(&buf[0]);
    return text;
}

int main(int argc, char** argv)
{
    std::string s = "Instrument label: %s";
    std::cout << formatstr(&s, "Frequency Generator") << "\n";

    return EXIT_SUCCESS;
}
benjarobin
  • 4,410
  • 27
  • 21