views:

7821

answers:

6

I am working with an open-source UNIX tool that is implemented in C++, and I need to change some code to get it to do what I want. I would like to make the smallest possible change in hopes of getting my patch accepted upstream. Solutions that are implementable in standard C++ and do not create more external dependencies are preferred.

Here is my problem. I have a C++ class -- let's call it "A" -- that currently uses fprintf() to print its heavily formatted data structures to a file pointer. In its print function, it also recursively calls the identically defined print functions of several member classes ("B" is an example). There is another class C that has a member std::string "foo" that needs to be set to the print() results of an instance of A. Think of it as a to_str() member function for A.

In pseudocode:

class A {
public:
  ...

  void print(FILE* f);
  B b;

  ...  
};

...

void A::print(FILE *f)
{
  std::string s = "stuff";
  fprintf(f, "some %s", s);
  b.print(f);
}

class C {
  ...
  std::string foo;
  bool set_foo(std::str);
  ...
}

...

A a = new A();
C c = new C();

...

// wish i knew how to write A's to_str()
c.set_foo(a.to_str());

I should mention that C is fairly stable, but A and B (and the rest of A's dependents) are in a state of flux, so the less code changes necessary the better. The current print(FILE* F) interface also needs to be preserved. I have considered several approaches to implementing A::to_str(), each with advantages and disadvantages:

  1. Change the calls to fprintf() to sprintf()

    • I wouldn't have to rewrite any format strings
    • print() could be reimplemented as: fprint(f, this.to_str());
    • But I would need to manually allocate char[]s, merge a lot of c strings , and finally convert the character array to a std::string
  2. Try to catch the results of a.print() in a string stream

    • I would have to convert all of the format strings to << output format. There are hundreds of fprintf()s to convert :-{
    • print() would have to be rewritten because there is no standard way that I know of to create an output stream from a UNIX file handle (though this guy says it may be possible).
  3. Use Boost's string format library

    • More external dependencies. Yuck.
    • Format's syntax is different enough from printf() to be annoying:

    printf(format_str, args) -> cout << boost::format(format_str) % arg1 % arg2 % etc

  4. Use Qt's QString::printf()

    • A different external dependency.
    • Looks like QString::printf() is on its way out

So, have I exhausted all possible options? If so, which do you think is my best bet? If not, what have I overlooked?

Thanks.

A: 

Is this about serialization? Or printing proper? If the former, consider boost::serialization as well. It's all about "recursive" serialization of objects and sub-object.

Assaf Lavie
This is about printing proper. C.foo is one piece of data that is eventually shown to the user (largely) as-is. If this were my code, I would lose the print(FILE*) nonsense which is far too restrictive.
underspecified
+5  A: 

I am using #3: the boost string format library - but I have to admit that I've never had any problem with the differences in format specifications.

Works like a charm for me - and the external dependencies could be worse (a very stable library)

Edited: adding an example how to use boost::format instead of printf:

sprintf(buffer, "This is a string with some %s and %d numbers", "strings", 42);

would be something like this with the boost::format library:

string = boost::str(boost::format("This is a string with some %s and %d numbers") %"strings" %42);

Hope this helps clarify the usage of boost::format

I've used boost::format as a sprintf / printf replacement in 4 or 5 applications (writing formatted strings to files, or custom output to logfiles) and never had problems with format differences. There may be some (more or less obscure) format specifiers which are differently - but I never had a problem.

In contrast I had some format specifications I couldn't really do with streams (as much as I remember)

bernhardrusch
Thanks for the clarification on boost::format's usage. It's tempting given that this project already depends on another boost library, but I don't think anything beats a printf that just works with std::string as Loki seems to do.
underspecified
I tried Loki's SafeFormat, but it turns out it's just swapping boost's 5s for ()s. On a positive note, my code did work once I adopted boost::format :-)
underspecified
Good to hear that boost::format worked for you - never tried the Loki method.
bernhardrusch
Got that hammer in my toolbox so I've got to use it somewhere...
Eric
+1  A: 

You can use std::string and iostreams with formatting, such as the setw() call and others in iomanip

Yann Ramin
Thanks for letting me know about iomanip. It certainly looks useful, but I want to try to preserve the numerous existing format strings as much as possible.
underspecified
+7  A: 

Here's the idiom I like for making functionality identical to 'sprintf', but returning a std::string, and immune to buffer overflow problems. This code is part of an open source project that I'm writing (BSD license), so everybody feel free to use this as you wish.

#include <string>
#include <cstdarg>
#include <vector>
#include <string>

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.
    size_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);
    // NB. On Windows, vsnprintf returns -1 if the string didn't fit the
    // buffer.  On Linux & OSX, it returns the length it would have needed.

    if (needed <= size && needed >= 0) {
        // 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]);
    }
}

EDIT: when I wrote this code, I had no idea that this required C99 conformance and that Windows (as well as older glibc) had different vsnprintf behavior, in which it returns -1 for failure, rather than a definitive measure of how much space is needed. Here is my revised code, could everybody look it over and if you think it's ok, I will edit again to make that the only cost listed:

std::string
Strutil::vformat (const char *fmt, va_list ap)
{
    // Allocate a buffer on the stack that's big enough for us almost
    // all the time.  Be prepared to allocate dynamically if it doesn't fit.
    size_t size = 1024;
    char stackbuf[1024];
    std::vector<char> dynamicbuf;
    char *buf = &stackbuf[0];

    while (1) {
        // Try to vsnprintf into our buffer.
        int needed = vsnprintf (buf, size, fmt, ap);
        // NB. C99 (which modern Linux and OS X follow) says vsnprintf
        // failure returns the length it would have needed.  But older
        // glibc and current Windows return -1 for failure, i.e., not
        // telling us how much was needed.

        if (needed <= (int)size && needed >= 0) {
            // It fit fine so we're done.
            return std::string (buf, (size_t) needed);
        }

        // vsnprintf reported that it wanted to write more characters
        // than we allotted.  So try again using a dynamic buffer.  This
        // doesn't happen very often if we chose our initial size well.
        size = (needed > 0) ? (needed+1) : (size*2);
        dynamicbuf.resize (size);
        buf = &dynamicbuf[0];
    }
}
Larry Gritz
Nice work on format/vformat. Maybe stackoverflow needs some kind of code snippit sharing section :-)
underspecified
It seems that your code will not work if the resulting string is larger then 1024 bytes. According to MSDN: vsnprintf - Return Value ... if the number of characters to write is greater than count, these functions return -1 indicating that output has been truncated.
Andreas
Bklyn
Also, use the return value from vsnprintf when creating your string. This will save you an extra strlen call.
Bklyn
Andreas: Thanks so much! I didn't realize that vsnprintf has a different failure return value on Windows. I will edit the code example to reflect this.
Larry Gritz
Bklyn: thanks for the tips, again related to vsnprintf that wasn't C99. Could you look over my edits and tell me what you think of the new version?
Larry Gritz
A: 

The following might be an alternative solution:

void A::printto(ostream outputstream) {
    char buffer[100];
    string s = "stuff";
    sprintf(buffer, "some %s", s);
    outputstream << buffer << endl;
    b.printto(outputstream);
}

(B::printto similar), and define

void A::print(FILE *f) {
    printto(ofstream(f));
}

string A::to_str() {
    ostringstream os;
    printto(os);
    return os.str();
}

Of course, you should really use snprintf instead of sprintf to avoid buffer overflows. You could also selectively change the more risky sprintfs to << format, to be safer and yet change as little as possible.

Jan de Vos
I will take your reply as a combination of #1 and #2 :-) Does ofstream have a constructor that takes file handles? I was under the impression that they were incompatible ...
underspecified
@underspecified: you were correct.
smerlin
+1  A: 

You should try the Loki library's SafeFormat header file (http://loki-lib.sourceforge.net/index.php?n=Idioms.Printf). It's similar to boost's string format library, but keeps the syntax of the printf(...) functions.

I hope this helps!

Kevin
Awesome! I think we have a new winner ^_^
underspecified
I gave SafeFormat a try, but I think the statement that it "keeps the syntax of the printf(...)" is misleading. All it appears to do is trade boost's %s for ()s. :-( I
underspecified