views:

56

answers:

4

I have this code inside a constructor of a class (not written by me) and it writes a variable arg list to a tmp file.

I wondered why this would be needed? The tmpfile is removed after this ctor goes out of scope and the var arg list sits inside the m_str vector.

Can someone suggest a better way of doing this without the use of a tmpfile?

DString(const char *fmt, ...)
    {

        DLog::Instance()->Log("Inside DString with ellipses");

        va_list varptr;
        va_start(varptr, fmt);
        FILE *f = tmpfile();
        if (f != NULL)
        {
            int n = ::vfprintf(f, fmt, varptr) + 1; 
            m_str.resize(n + 1);
            ::vsprintf(&m_str[0], fmt, varptr);
            va_end(varptr);
        }
        else
            DLog::Instance()->Log("[ERROR TMPFILE:] Unable to create TmpFile for request!");
    }
+1  A: 

It's just using the temporary file as a place that it can write the contents that won't overflow, so it can measure the length, then allocate sufficient space for the string, and finally deposit the real output in the string.

I'd at least consider how difficult it would be to replace the current printf-style interface that's leading to this with an iostreams-style interface, which will make it easy to avoid and give all the usual benefits of iostreams (type-safe, extensible, etc.)

Edit: if changing the function's signature is really too difficult to contemplate, then you probably want to replace vfprintf with vsnprintf. vsnprintf allows you to specify a buffer length (so it won't overrun the buffer) and it returns the number of characters that would have been generated if there had been sufficient space. As such, usage would be almost like you have now, but avoid generating the temporary file. You'd call it once specifying a buffer length of 0, use the return value (+1 for the NUL terminator) to resize your buffer, then call it again specifying the correct buffer size.

Jerry Coffin
Is it replaceable without changing this contructor's signature?
Tony
@Tony: yes. `n = vsnprintf(0, 0, fmt, varptr) + 1` will give you the length without flapping around with temporary files.
Mike Seymour
A: 

It appears to be using the temp file as an output place for the ::vfprintf() call. It does that to get the length of the formatted string (plus 1 for the NULL). Then resizes m_str big enough to hold the formatted string, which gets filled in from the ::vsprintf() call.

The var arg list is not in the file or in m_str. The formatted output from printf() (and its variants) is in the file and in m_str.

Scott Thomson
A: 

I have a queasy feeling showing this but you could try:

  FILE *fp=freopen("nul","w", stderr)
  int n = ::vfprintf( fp , fmt, varptr );
  fclose(fp);

(windows)

Anders K.
+2  A: 

This is C++ code: I think you may be trying to solve the wrong problem here.

The need for a temp file would go away completely if you consider using a C++-esque design instead of continuing to use the varargs. It may seem like a lot of work to convert all the calling sites to use a new mechanism, but varargs provide a wide variety of possibilities to mis-pass parameters leaving you open to insidious bugs, not to mention you can't pass non-POD types at all. I believe in the long (or even medium) term it will pay off in reliability, clarity, and ease of debugging.

Instead try to implement a C++-style streams interface that provides type safety and even the ability to disallow certain operations if needed.

Mark B