CODEWITHSUNDEEP
c++multithreadingexceptionstring-building
Matthias Ronge
Matthias Ronge

Reputation: 10112

String building in C++ exception’s what()

This answer declares a private static ostringstream. Is this thread safe? If two threads throw (and catch, and log what()) the exception at the same time, does this work reliably? If I declare the ostringstream locally, like:

virtual const char* what() const throw()
{
    std::ostringstream cnvt.str( "" );
    cnvt << runtime_error::what() << ": " << getNumerator()
         << " / " << getDenominator();
    return cnvt.str().c_str();
}

Is there a drawback (memory leak, or illegal pointer)? Or is this the thread-safe way?

Upvotes: 1

Views: 363

Answers (2)

Richard Hodges
Richard Hodges

Reputation: 69864

what() is the wrong place to build the string IMHO (although there are different views on this).

std::runtime_error already contains a string, so let's use that one.

#include <stdexcept>
#include <string>

struct DivideByZero : std::runtime_error
{

    DivideByZero(int x, int y)
    : std::runtime_error( make_message(x,y) )
    {}

private:
    static std::string make_message(int x, int y)
    {
        return std::string("division by zero: " + std::to_string(x) + '/' + std::to_string(y));
    }

};

Upvotes: 1

David Haim
David Haim

Reputation: 26476

No. it's not safe at all (and pretty inefficient to my taste, can be done with std::string alone).
in order to make it safe, declare the ostringstream as thread_local

static thread_local ostringstream cnvt;

also, you should make the cnvt output the string to some member string in order to not return a dangling pointer. 

class DivideByZeroException: public runtime_error {
public:

  DivideByZeroException(int x, int y)
    : runtime_error( "division by zero" ), numerator( x ), denominator( y )
    {}

  virtual const char* what() const throw()
  {
    cnvt.str( "" );

    cnvt << runtime_error::what() << ": " << getNumerator()
         << " / " << getDenominator();

    error = cnvt.str();
    return error.c_str();
  } 

   /*...*/

   private:
     std::string error;
     int numerator;
     int denominator;

   static thread_local ostringstream cnvt;
};

also, if the exception is "divide by zero" don't you think it's a bit silly storing the denominator? it's always a zero! otherwise you wouldn't throw "division by zero" error!

and finally 0-division error is more appropriate to be derived from std::domain_error which related to mathematical errors.

Upvotes: 4

Related Questions