1

Based on the answers here to the question of how to format numbers with a comma, I am using the following code:

#include <locale>
#include <stringstream>

namespace
{

class comma_numpunct : public std::numpunct<char>
{
  protected:
    virtual char do_thousands_sep() const
    {
        return ',';
    }

    virtual std::string do_grouping() const
    {
        return "\03";
    }
};

}


/* Convert number to string using a comma as the thousands separator. */
string thousands(const int x) {

    /* custom locale to ensure thousands separator is comma */
    comma_numpunct* comma_numpunct_ptr = new comma_numpunct();
    std::locale comma_locale(std::locale(), comma_numpunct_ptr);

    stringstream ss;
    ss.imbue(comma_locale); // apply locale to stringstream
    ss << x;

    /* garbage collection */
    delete comma_numpunct_ptr;

    return ss.str();
}

GDB gives the following backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000000021 in ?? ()
(gdb) bt
#0  0x0000000000000021 in ?? ()
#1  0x00007ffff7701535 in std::locale::_Impl::~_Impl() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#2  0x00007ffff770166d in std::locale::~locale() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x000000000044b93e in thousands (x=358799) at ../globals_utilities.cpp:104
#4  0x000000000045650d in main (argc=1, argv=0x7fffffffdf58) at ../main.cpp:67

So, the problem is (I believe) with my attempt at freeing the new'd memory. But I don't know how to work around this. (I can't use std::unique_ptr because I'm not always compiling with C++11 support.)

How can I fix the segfault without leaking memory?

Community
  • 1
  • 1
synaptik
  • 8,971
  • 16
  • 71
  • 98
  • 1
    You're not supposed to free the memory manually. The destructor of `std::locale` will do that for you. – David G Mar 25 '14 at 22:30
  • 1
    why not just use a static object? No need to delete anything then. – Deduplicator Mar 25 '14 at 22:31
  • @user4815162342 Still sigfaults. – synaptik Mar 25 '14 at 22:34
  • 1
    You may find the Note on Option 7 for [`std::locale::locale()`](http://en.cppreference.com/w/cpp/locale/locale/locale) informative: "Overload 7 is typically called with its second argument, f, obtained directly from a new-expression: **the locale is responsible for calling the matching delete from its own destructor.**" In other words, don't delete what you no longer own. – WhozCraig Mar 25 '14 at 22:36

1 Answers1

2

Your problem is the locale facet (numpunct). If you pass it to a locale via constructor and the references of the facet is zero the locale will delete the facet.

You might do:

comma_numpunct(size_t refs = 0)
:   numpunct(refs)
{}

and

comma_numpunct* comma_numpunct_ptr = new comma_numpunct(1);

or better omit:

// double delete
// delete comma_numpunct_ptr;

You may omit the allocation of the facet:

string thousands(const int x) {
   comma_numpunct numpunct(1);
   std::locale comma_locale(std::locale(), &numpunct);
   std::stringstream ss;
   ss.imbue(comma_locale);
   ss << x;
   return ss.str();
}

From 22.3.1.1.2 Class locale::facet

The refs argument to the constructor is used for lifetime management.

— For refs == 0, the implementation performs delete static_cast(f) (where f is a pointer to the facet) when the last locale object containing the facet is destroyed; for refs == 1, the implementation never destroys the facet.