On 09/10/2012 10:56 AM, Stefan Teleman wrote:
On Thu, Sep 6, 2012 at 6:43 PM, Stefan Teleman<[email protected]> wrote:
On Thu, Sep 6, 2012 at 4:02 PM, Martin Sebor<[email protected]> wrote:
Here's a thought: it's not pretty but how about having
the function initialize the facet? It already has a pointer
to the base class, so it could downcast it to std::numpunct
(or moneypunct, respectively), and assign the initial values
to the members. Would that work?
I haven't looked at them in detail (yet) but a cursory look shows that
they're both recursive for the successful case.
It's not going to work that way.
For one, __rw_get_numpunct() and __rw_get_moneypunct() are static
functions in the __rw namespace. Neither can access or modify the
std::numpunct<T> or std::moneypunct<T> data members directly, because
they are private.
There are ways around it -- see, for example, the hack in
src/access.h. This gives us, the implementation, a way to
cleanly access private data without exposing them to user
programs. In our case, we could solve our problem by having
the facet declare some helper (defined only in punct.cpp)
a friend and invoking the helper from __rw_get_numpunct.
That said, I'd certainly prefer to avoid hacks as much as
possible. This problem could perhaps more cleanly be solved
by having the facet pass a reference to the string (or to
all of its internal data) to modify to the function (or
something like that). Unfortunately, it would break binary
compatibility.
Second, both __rw_get_numpunct() and __rw_get_moneypunct() are
recursive. Unless we want to start playing with
PTHREAD_MUTEX_RECURSIVE, which I'm not at all sure is supported on all
the platforms we support, we're not going to be able to solve the
thread-safety problem here (Linux supports it as
PTHREAD_MUTEX_RECURSIVE_NP, Solaris supports it
PTHREAD_MUTEX_RECURSIVE).
If I remember correctly, the recursion is quite simple. The
first "stage" (pfacet->_C_data () != 0) sets up the internal
data and takes place just once per facet object. The second
(recursive) stage then extracts and returns it. I only looked
at it briefly last week but from what I saw I'd say it should
be possible to lock only the second, non-recursive stage and
do the assignment there.
Third, both __rw_get_numpunct() and __rw_get_moneypunct() can return a
NULL pointer. This is bad, because it will cause a SEGV at string
assignment, during a call to either of the
std::numpunct<T>::grouping(), std::numpunct<T>::truename(), etc.
functions. We should fix this and throw an exception instead. The
Standard doesn't say that any of these functions can throw, but it
doesn't say they can't throw either. And both __rw_get_numpunct() and
__rw_get_moneypunct() throw already.
The functions only return NULL on internal error (i.e., a bug
in the implementation). There's an assertion right above the
return statement that will fire in a debug build. There's no
point in throwing an exception to the user for our own bugs
(they would have no way to recover from it). It's better to
abort. We might want to do that (i.e., abort) unconditionally
in this case, even when assertions are disabled.
Martin
--Stefan
