stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <mse...@gmail.com>
Subject Re: STDCXX-1056 [was: Re: STDCXX forks]
Date Mon, 10 Sep 2012 17:32:48 GMT
On 09/10/2012 10:56 AM, Stefan Teleman wrote:
> On Thu, Sep 6, 2012 at 6:43 PM, Stefan Teleman<stefan.teleman@gmail.com>  wrote:
>> On Thu, Sep 6, 2012 at 4:02 PM, Martin Sebor<msebor@gmail.com>  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
>


Mime
View raw message