From stdcxx-dev-return-4318-apmail-incubator-stdcxx-dev-archive=incubator.apache.org@incubator.apache.org Tue Aug 14 01:21:57 2007 Return-Path: Delivered-To: apmail-incubator-stdcxx-dev-archive@www.apache.org Received: (qmail 42955 invoked from network); 14 Aug 2007 01:21:56 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 14 Aug 2007 01:21:56 -0000 Received: (qmail 1945 invoked by uid 500); 14 Aug 2007 01:21:55 -0000 Delivered-To: apmail-incubator-stdcxx-dev-archive@incubator.apache.org Received: (qmail 1935 invoked by uid 500); 14 Aug 2007 01:21:55 -0000 Mailing-List: contact stdcxx-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: stdcxx-dev@incubator.apache.org Delivered-To: mailing list stdcxx-dev@incubator.apache.org Received: (qmail 1923 invoked by uid 99); 14 Aug 2007 01:21:55 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 13 Aug 2007 18:21:55 -0700 X-ASF-Spam-Status: No, hits=-0.0 required=10.0 tests=SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: local policy) Received: from [208.30.140.160] (HELO moroha.quovadx.com) (208.30.140.160) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 14 Aug 2007 01:22:04 +0000 Received: from qxvcexch01.ad.quovadx.com ([192.168.170.59]) by moroha.quovadx.com (8.13.6/8.13.6) with ESMTP id l7E1LSLY016196 for ; Tue, 14 Aug 2007 01:21:28 GMT X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results [take 2] Date: Mon, 13 Aug 2007 19:20:44 -0600 Message-ID: In-Reply-To: <46BFD340.5020701@roguewave.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results [take 2] Thread-Index: AcfdXDC3agccTU/+R16FxGNHrRYlcAAs8XYg References: <46BFD340.5020701@roguewave.com> From: "Travis Vitek" To: X-Virus-Checked: Checked by ClamAV on apache.org =20 >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 =3D 0; i !=3D rw_opt_nloops; ++i) { >> =20 >> - io.width (i % 16); >> + // fill in the value and results for this locale >> + const MyNumData& data =3D my_num_data [i % nlocales]; >> =20 >> - // exercise postive and negative values >> - const int ival =3D 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 =3D >> + rw_opt_shared_locale ? data.locale_ >> + : std::locale (data.locale_name_); >> if (test_char) { >> // exercise the narrow char specialization of the facet >> =20 >> const std::num_put &np =3D >> std::use_facet >(loc); >> =20 >> - const std::ostreambuf_iterator 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. >> =20 >[...] >> +#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(&sb), \ >> + io, fill, (valueT)(val)) =3D charT (); \ >> + RW_ASSERT (!io.fail ()); \ >> + RW_ASSERT (!rw_strncmp (buf, cmp)); >> =20 >> - 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) >> =20 >> - case put_ulong: >> - np.put (iter, io, ' ', (unsigned long)ival); >> - break; >> - >> + TEST_N (data.bool_, bool, data.value_ !=3D 0);=20 >> + 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. >> =20 >[...] >> +#define SETUP(sb, buf, io, p, fill, valueT, val, charT, loc) \ >> + sb.pubsetp (buf, countof (buf)); \ >> + io.rdbuf (&sb); \ >> + *p.put (std::ostreambuf_iterator(&sb), \ >> + io, fill, (valueT)(val)) =3D 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.