views:

1639

answers:

4

A previous question showed a nice way of printing to a string. The answer involved va_copy:

std::string format (const char *fmt, ...);
{
   va_list ap;
   va_start (ap, fmt);
   std::string buf = vformat (fmt, ap);
   va_end (ap);
   return buf;
}


std::string vformat (const char *fmt, va_list ap)
{
   // Allocate a buffer on the stack that's big enough for us almost
   // all the time.
   s ize_t size = 1024;
   char buf[size];

   // Try to vsnprintf into our buffer.
   va_list apcopy;
   va_copy (apcopy, ap);
   int needed = vsnprintf (&buf[0], size, fmt, ap);

   if (needed <= size) {
       // It fit fine the first time, we're done.
       return std::string (&buf[0]);
   } else {
       // vsnprintf reported that it wanted to write more characters
       // than we allotted.  So do a malloc of the right size and try again.
       // This doesn't happen very often if we chose our initial size
       // well.
       std::vector <char> buf;
       size = needed;
       buf.resize (size);
       needed = vsnprintf (&buf[0], size, fmt, apcopy);
       return std::string (&buf[0]);
   }

}

The problem I'm having is that the above code doesn't port to Visual C++ because it doesn't provide va_copy (or even __va_copy). So, does anyone know how to safely port the above code? Presumably, I need to do a va_copy copy because vsnprintf destructively modifies the passed va_list.

+1  A: 

Varargs is not a nice way to print a string. In C++, it was the least terrible solution there was.

The recommended way is to use the C++ iostreams library. If you absolutely want printf-like formatting and syntax, you can use boost::format with it, which preserves type safety and works with C++ types.

And finally, if you insist on throwing away type safety and limiting yourself to C types, you already have the printf family of functions in the language, and there is no need to reinvent them.

jalf
Well, I'm not planning on reimplementing existing code and bringing in new libraries (with their own portability problems), just to port some legacy code.The printf family of functions do not print text to a string. You'll notice the above wrap calls vsprintf to overcome the fixed buffer size.
Perhaps you could tell me what portability problems boost::format has? ;)
jalf
By contrast, what you're doing is full of portability problems. For starters, relying on C99 functionality in C++ is not a good idea. If you need string formatting, why not use the tools C++ make available for the task? Is the printf syntax so essential that going to all this trouble is worth it?
jalf
A: 

I've implemented similar things in VC++ and have never needed to use va_copy(). What happens when you try it without using the copy?

Greg Hewgill
Who knows...It may appear to work. Even if it does, it doesn't mean its safe.
Apparently va_copy() is a C99 thing. For VC++, you will be just fine using the original va_list more than once without worrying about a copy. vsnprintf won't try to modify the passed list.
Greg Hewgill
Practically speaking, copying it without va_copy is fine because a va_list in Visual C++ is just a pointer to the next argument, and calling vsnprintf can't modify the caller's value. Technically, though, ap's value after the first call is indeterminate, so you shouldn't use it for a second call.
Rob Kennedy
+5  A: 

You should be able to get away with just doing a regular assignment:

va_list apcopy = ap;

It's technically non-portable and undefined behavior, but it will work with most compilers and architectures. In the x86 calling convention, va_lists are just pointers into the stack and are safe to copy.

Adam Rosenfield
True - va_copy is commonly defined as "#define va_copy(d,s) ((d) = (s))", so it might be best just to throw that into a 'portability' header protected by an "#ifndef va_copy"
Michael Burr
+4  A: 

One thing you can do is if you do not otherwise need the vformat() function, move its implementation into the format() function (untested):

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


std::string format(const char *fmt, ...)
{
   va_list ap;

   enum {size = 1024};

   // if you want a buffer on the stack for the 99% of the time case 
   //   for efficiency or whatever), I suggest something like
   //   STLSoft's auto_buffer<> template.
   //
   //   http://www.synesis.com.au/software/stlsoft/doc-1.9/classstlsoft_1_1auto__buffer.html
   //
   std::vector<char> buf( size);

   //
   // where you get a proper vsnprintf() for MSVC is another problem
   // maybe look at http://www.jhweiss.de/software/snprintf.html
   //

   // note that vsnprintf() might use the passed ap with the 
   //   va_arg() macro.  This would invalidate ap here, so we 
   //   we va_end() it here, and have to redo the va_start()
   //   if we want to use it again. From the C standard:
   //
   //       The object ap may be passed as an argument to
   //       another function; if that function invokes the 
   //       va_arg macro with parameter ap, the value of ap 
   //       in the calling function is indeterminate and 
   //       shall be passed to the va_end macro prior to 
   //       any further reference to ap.   
   //
   //    Thanks to Rob Kennedy for pointing that out.
   //
   va_start (ap, fmt);
   int needed = vsnprintf (&buf[0], buf.size(), fmt, ap);
   va_end( ap);

   if (needed >= size) {
       // vsnprintf reported that it wanted to write more characters
       // than we allotted.  So do a malloc of the right size and try again.
       // This doesn't happen very often if we chose our initial size
       // well.
       buf.resize( needed + 1);

       va_start (ap, fmt);
       needed = vsnprintf (&buf[0], buf.size(), fmt, ap);
       va_end( ap);

       assert( needed < buf.size());
   }

   return std::string( &buf[0]);
}
Michael Burr
You'd have to call va_end and va_start again before the second call to vsnprintf, wouldn't you?
Rob Kennedy
Hmm - looks like you're right. I never realized that before. -- Fixed (I think).
Michael Burr