>Martin Sebor wrote:
>
>Travis Vitek wrote:
>> Attached is a patch to enhance the num_put facet mt test. Threads
>> verify that the values they put compare equal to those put in the
>> primary thread.
>
>There are few outstanding issues here that we need to resolve before
>committing this patch...
>
>[...]
>> + for (int i = 0; i != rw_opt_nloops; ++i) {
>>
>> - io.width (i % 16);
>> + // fill in the value and results for this locale
>> + const MyNumData& data = my_num_data [i % nlocales];
>>
>> - // exercise postive and negative values
>> - const int ival = i & 1 ? -i : i;
>> -
>> + // construct a named locale and imbue it in the ios object
>> + // so that the locale is used not only by the num_put facet
>> + const std::locale loc =
>> + rw_opt_shared_locale ? data.locale_
>> + : std::locale (data.locale_name_);
>> if (test_char) {
>> // exercise the narrow char specialization of the facet
>>
>> const std::num_put<char> &np =
>> std::use_facet<std::num_put<char> >(loc);
>>
>> - const std::ostreambuf_iterator<char> iter (&sb);
>> + nio.imbue (loc);
>
>Calling imbue() on the ios object before calling rdbuf() on it prevents
>the same locale from being imbued in the streambuf. Did you intend for
>that to happen?
>
No. I will double-check this is working as intended and update the
necessary tests if it is not.
>>
>[...]
>> +#define TEST(sb, buf, cmp, io, p, fill, valueT, val, charT) \
>> + sb.pubsetp (buf, countof (buf)); \
>> + io.rdbuf (&sb); \
>
>Is the call to rdbuf() necessary for every test or can it be done
>just once per thread? (Preferably before calling imbue() on the ios
>object.)
>
The rdbuf() isn't necessary every call. I'll update to call imbue() on
the ios object one time per iteration.
>> + *p.put (std::ostreambuf_iterator<charT>(&sb), \
>> + io, fill, (valueT)(val)) = charT (); \
>> + RW_ASSERT (!io.fail ()); \
>> + RW_ASSERT (!rw_strncmp (buf, cmp));
>>
>> - case put_long:
>> - np.put (iter, io, ' ', long (ival));
>> - break;
>> +#define TEST_N(o, t, v) \
>> + TEST(nsb, ncs, o.ncs_, nio, np, ' ', t, v, char)
>>
>> - case put_ulong:
>> - np.put (iter, io, ' ', (unsigned long)ival);
>> - break;
>> -
>> + TEST_N (data.bool_, bool, data.value_ != 0);
>> + TEST_N (data.long_, long, data.value_);
>> + TEST_N (data.ulong_, unsigned long, data.value_);
>
>I note you've changed the test from invoking one num_put member
>per iteration to invoking all members each iteration. I'm curious
>about your rationale for the change? (I don't necessarily disagree
>with the approach, just wondering what if anything led you to make
>the change.) FWIW, the one advantage to sticking with the original
>testing strategy is that it would let us eliminate the call to
>pubsetp() and (possibly) do away with the macros.
I'm missing something here. I don't see how we can eliminate the
pubsetp() calls. If we did, then the next insertion operation will
insert after the previous one. Instead of writing to the front of the
buffer every time, we'd be appending to it. At some point an insertion
will fail because the put pointer is at the end. Am I wrong?
The original test had dummy streambufs that did no actual insertion, so
this wasn't an issue.
>>
>[...]
>> +#define SETUP(sb, buf, io, p, fill, valueT, val, charT, loc) \
>> + sb.pubsetp (buf, countof (buf)); \
>> + io.rdbuf (&sb); \
>> + *p.put (std::ostreambuf_iterator<charT>(&sb), \
>> + io, fill, (valueT)(val)) = charT (); \
>> + rw_assert (!io.fail (), __FILE__, __LINE__, \
>
>I don't think rw_assert() is appropriate here. If the facet fails
>to format the "master" value into the buffer here the rest of the
>test is most likely hosed anyway and shouldn't even be attempted.
>Wouldn't rw_fatal() be a better choice?
Agreed
>
>Also, if you'd like to get rid of the macro (I would :), it should
>be possible to move the call to rdbuf() outside the loop and verify
>that the formatting was successful only once per iteration.
>
Done.
