You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Travis Vitek <tv...@quovadx.com> on 2007/08/06 23:52:06 UTC

[PATCH] Update test 22.locale.num.put.mt.cpp to validate results

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.

2007-08-06	Travis Vitek	<vi...@roguewave.com>

	* 22.locale.num.put.mt.cpp: Added structure for MyIos,
MyStreambuf
	and CnumData types to simplify testing.
	(thread_func): Verify that data written matches expected.
	(run_test): Build table of inputs and outputs for verification
by
	test threads.

Index: 22.locale.num.put.mt.cpp
===================================================================
--- 22.locale.num.put.mt.cpp	(revision 562577)
+++ 22.locale.num.put.mt.cpp	(working copy)
@@ -36,6 +36,8 @@
 #include <rw_thread.h>   // for rw_get_processors (), rw_thread_pool()
 #include <driver.h>
 
+// helper gets the number of elements in an array
+#define countof(x) (sizeof(x) / sizeof(*x))
 
 // maximum number of threads allowed by the command line interface
 #define MAX_THREADS   32
@@ -58,8 +60,98 @@
 static std::size_t
 nlocales;
 
+struct CnumData
+{
+    enum { BufferSize = 32 };
+
+    // name of the locale the data corresponds to
+    const char* locale_name_;
+
+    // optinally set to the named locale for threads to share
+    std::locale locale_;
+
+    // holder for original value and character representation
+    template <typename T>
+    struct result
+    {
+        result()
+            : flags_(std::ios::dec)
+        {
+            memset(nrep_, 0, sizeof nrep_);
+            memset(wrep_, 0, sizeof wrep_);
+        }
+
+        T value_; // the original value
+        std::ios::fmtflags flags_; // flags used
+
+        // holds the narrow/wide character representation of value_
+        char    nrep_[BufferSize]; 
+        wchar_t wrep_[BufferSize];
+    };
+
+    int width_;
+
+    result<bool> bool_;
+    result<long> long_;
+    result<unsigned long> ulong_;
+
+#ifndef _RWSTD_NO_LONG_LONG
+
+    result<_RWSTD_LONG_LONG> llong_;
+    result<unsigned _RWSTD_LONG_LONG> ullong_;
+
+#endif // _RWSTD_NO_LONG_LONG
+
+    result<double> dbl_;
+
+#ifndef _RWSTD_NO_LONG_DOUBLE
+
+    result<long double> ldbl_;
+
+#endif // _RWSTD_NO_LONG_DOUBLE
+
+    result<void*> ptr_;
+
+} num_put_data[MAX_THREADS];
+
+const std::ios::fmtflags baseflags[] = {
+    std::ios::oct,
+    std::ios::dec,
+    std::ios::hex
+};
+
+const std::ios::fmtflags fmtflags[] = {
+    std::ios::showpos,
+    std::ios::showpoint,
+    std::ios::fixed,
+    std::ios::scientific
+};
+
+const std::ios::fmtflags adjustflags[] = {
+    std::ios::internal,
+    std::ios::left,
+    std::ios::right
+};
+
 
/***********************************************************************
***/
 
+template <class charT, class Traits>
+struct MyIos: std::basic_ios<charT, Traits>
+{
+    MyIos () { this->init (0); }
+};
+
+template <class charT, class Traits>
+struct MyStreambuf: std::basic_streambuf<charT, Traits>
+{
+    typedef std::basic_streambuf<charT, Traits> Base;
+
+    MyStreambuf (charT *pbeg, charT *pend)
+        : Base () {
+        this->setp (pbeg, pend);
+    }
+};
+
 extern "C" {
 
 bool test_char;    // exercise num_put<char>
@@ -69,54 +161,43 @@
 static void*
 thread_func (void*)
 {
-    // dummy streambuf-derived object the doesn't do anything
-    // but allows ostreambuf_iterator to "think" it can write
-    // to it
-    struct NarrowBuf: std::streambuf {
-        int_type overflow (int_type c) { return c; }
-    } sb;
 
-#ifndef _RWSTD_NO_WCHAR_T
+// these macros should probably assert goodbit
 
-    struct WideBuf: std::wstreambuf {
-        int_type overflow (int_type c) { return c; }
-    } wb;
+#define TEST_PUT(cType, vType, field, value, fill, flag)            \
+    {                                                               \
+        MyIos<cType, std::char_traits<cType> > io;                  \
+        io.imbue(loc); io.flags(flag); io.width(data->width_);      \
+                                                                    \
+        cType chars[CnumData::BufferSize];                          \
+        memset(chars, 0, sizeof chars);                             \
+                                                                    \
+        MyStreambuf<cType, std::char_traits<cType> > sb             \
+           (chars, chars + CnumData::BufferSize);                   \
+                                                                    \
+        const std::ostreambuf_iterator<cType> iter (&sb);           \
+                                                                    \
+        np.put(iter, io, fill, value);                              \
+        rw_assert(0 == memcmp(field, chars, sizeof chars),          \
+            __FILE__, __LINE__,                                     \
+            "%s '%s' != '%s'", #vType, field, chars);               \
+    }
 
-#endif   // _RWSTD_NO_WCHAR_T
+#define TEST_PUT_NARROW(type, it)                                   \
+    TEST_PUT(char, type, it.nrep_, it.value_, ' ', it.flags_)
 
-    struct Ios: std::ios {
-        Ios () { this->init (0); }
-    } io;
+#define TEST_PUT_WIDE(type, it)                                     \
+    TEST_PUT(wchar_t, type, it.wrep_, it.value_, L' ', it.flags_)
 
-    const std::ios::fmtflags baseflags[] = {
-        std::ios::oct,
-        std::ios::dec,
-        std::ios::hex
-    };
-
-    const std::ios::fmtflags fmtflags[] = {
-        std::ios::showpos,
-        std::ios::showpoint,
-        std::ios::fixed,
-        std::ios::scientific
-    };
-
-    const std::ios::fmtflags adjustflags[] = {
-        std::ios::internal,
-        std::ios::left,
-        std::ios::right
-    };
-
     for (int i = 0; i != rw_opt_nloops; ++i) {
 
-        // save the name of the locale
-        const char* const locale_name = locales [i % nlocales];
+        // fill in the value and results for this locale
+        const CnumData* data = num_put_data + (i % nlocales);
 
         // construct a named locale and imbue it in the ios object
         // so that the locale is used not only by the num_put facet
         // but also by the numpunct facet
-        const std::locale loc (locale_name);
-        io.imbue (loc);
+        const std::locale loc (data->locale_name_);
 
         enum PutId {
             put_bool,
@@ -131,75 +212,62 @@
 #endif   // _RWSTD_NO_LONG_LONG
 
             put_dbl,
+
+#ifndef _RWSTD_NO_LONG_DOUBLE
+
             put_ldbl,
+
+#endif // _RWSTD_NO_LONG_DOUBLE
+
             put_ptr,
             put_max
         };
 
-        const std::ios::fmtflags base =
-            baseflags [i % (sizeof baseflags / sizeof *baseflags)];
-
-        const std::ios::fmtflags fmt =
-            fmtflags [i % (sizeof baseflags / sizeof *baseflags)];
-
-        const std::ios::fmtflags adjust =
-            adjustflags [i % (sizeof baseflags / sizeof *baseflags)];
-
-        io.flags (base | fmt | adjust);
-
-        io.width (i % 16);
-
-        // exercise postive and negative values
-        const int ival = i & 1 ? -i : i;
-
         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);
-
             switch (i % put_max) {
             case put_bool:
-                if (i & 2)
-                    io.setf (std::ios::boolalpha);
-                else
-                    io.unsetf (std::ios::boolalpha);
-                        
-                np.put (iter, io, ' ', bool (ival));
+                TEST_PUT_NARROW(bool, data->bool_);
                 break;
 
             case put_long:
-                np.put (iter, io, ' ', long (ival));
+                TEST_PUT_NARROW(long, data->long_);
                 break;
 
             case put_ulong:
-                np.put (iter, io, ' ', (unsigned long)ival);
+                TEST_PUT_NARROW(unsigned long, data->ulong_);
                 break;
 
 #ifndef _RWSTD_NO_LONG_LONG
 
             case put_llong:
-                np.put (iter, io, ' ', (_RWSTD_LONG_LONG)ival);
+                TEST_PUT_NARROW(long long, data->llong_);
                 break;
 
-#endif   // _RWSTD_NO_LONG_LONG
-
             case put_ullong:
-                np.put (iter, io, ' ', (unsigned
_RWSTD_LONG_LONG)ival);
+                TEST_PUT_NARROW(unsigned long long, data->ullong_);
                 break;
 
+#endif   // _RWSTD_NO_LONG_LONG
+
             case put_dbl:
-                np.put (iter, io, ' ', double (ival));
+                TEST_PUT_NARROW(double, data->dbl_);
                 break;
 
+#ifndef _RWSTD_NO_LONG_DOUBLE
+
             case put_ldbl:
-                np.put (iter, io, ' ', (long double)ival);
+                TEST_PUT_NARROW(long double, data->ldbl_);
                 break;
 
+#endif // _RWSTD_NO_LONG_DOUBLE
+
             case put_ptr:
-                np.put (iter, io, ' ', (void*)ival);
+                TEST_PUT_NARROW(void*, data->ptr_);
                 break;
             }
         }
@@ -214,43 +282,45 @@
             const std::num_put<wchar_t> &np =
                 std::use_facet<std::num_put<wchar_t> >(loc);
 
-            const std::ostreambuf_iterator<wchar_t> iter (&wb);
-
             switch (i % put_max) {
             case put_bool:
-                np.put (iter, io, L' ', bool (ival));
+                TEST_PUT_WIDE(bool, data->bool_);
                 break;
 
             case put_long:
-                np.put (iter, io, L' ', long (ival));
+                TEST_PUT_WIDE(long, data->long_);
                 break;
 
             case put_ulong:
-                np.put (iter, io, L' ', (unsigned long)ival);
+                TEST_PUT_WIDE(unsigned long, data->ulong_);
                 break;
 
 #ifndef _RWSTD_NO_LONG_LONG
 
             case put_llong:
-                np.put (iter, io, L' ', (_RWSTD_LONG_LONG)ival);
+                TEST_PUT_WIDE(long, data->llong_);
                 break;
 
             case put_ullong:
-                np.put (iter, io, L' ', (unsigned
_RWSTD_LONG_LONG)ival);
+                TEST_PUT_WIDE(unsigned long long, data->ullong_);
                 break;
 
 #endif   // _RWSTD_NO_LONG_LONG
 
             case put_dbl:
-                np.put (iter, io, L' ', double (ival));
+                TEST_PUT_WIDE(double, data->dbl_);
                 break;
 
+#ifndef _RWSTD_NO_LONG_DOUBLE
+
             case put_ldbl:
-                np.put (iter, io, L' ', (long double)ival);
+                TEST_PUT_WIDE(long double, data->ldbl_);
                 break;
 
+#endif // _RWSTD_NO_LONG_DOUBLE
+
             case put_ptr:
-                np.put (iter, io, L' ', (void*)ival);
+                TEST_PUT_WIDE(void*, data->ptr_);
                 break;
             }
 
@@ -273,12 +343,124 @@
     const char* const locale_list =
         rw_opt_locales ? rw_opt_locales : rw_locales (_RWSTD_LC_ALL);
 
-    const std::size_t maxinx = sizeof locales / sizeof *locales;
+    const std::size_t maxinx = countof(locales);
 
     for (const char *name = locale_list; *name; name += std::strlen
(name) +1) {
 
-        locales [nlocales++] = name;
+        const std::size_t inx = nlocales;
+        locales [inx] = name;
 
+        // fill in the value and results for this locale
+        CnumData* data = num_put_data + nlocales;
+        data->locale_name_ = name;
+
+        try {
+            const std::locale loc(name);
+
+            std::ios::fmtflags base =
+                baseflags [nlocales % countof(baseflags)];
+
+            std::ios::fmtflags fmt =
+                fmtflags [nlocales % countof(baseflags)];
+
+            std::ios::fmtflags adjust =
+                adjustflags [nlocales % countof(baseflags)];
+
+            data->width_ = nlocales % 16;
+
+            // exercise postive and negative values
+            const int ival = nlocales & 1 ? -1 * nlocales : nlocales;
+
+            // format data into buffers
+            const std::num_put<char> &np =
+                std::use_facet<std::num_put<char> >(loc);
+
+#ifndef _RWSTD_NO_WCHAR_T
+
+            const std::num_put<wchar_t> &wp =
+                std::use_facet<std::num_put<wchar_t> >(loc);
+
+#endif // _RWSTD_NO_WCHAR_T
+
+#define PUT(charT, field, value, fill, flag, putter)               \
+    {                                                              \
+        flag = base | fmt | adjust;                                \
+                                                                   \
+        MyIos<charT, std::char_traits<charT> > io;                 \
+        io.imbue(loc); io.flags(flag); io.width(data->width_);     \
+                                                                   \
+        MyStreambuf<charT, std::char_traits<charT> > sb(           \
+            field, field + CnumData::BufferSize);                  \
+        const std::ostreambuf_iterator<charT> iter (&sb);          \
+                                                                   \
+        putter.put(iter, io, fill, value);                         \
+    }
+
+#if 1
+#  define PUT_NARROW(it)                                           \
+    PUT(char, it.nrep_, it.value_, ' ', it.flags_, np)
+#else
+#  define PUT_NARROW(thing)
+#endif // 1
+
+#ifndef _RWSTD_NO_WCHAR_T
+#  define PUT_WIDE(it)                                             \
+    PUT(wchar_t, it.wrep_, it.value_, L' ', it.flags_, wp)
+#else
+#  define PUT_WIDE(thing)
+#endif // _RWSTD_NO_WCHAR_T
+
+            data->long_.value_   = (long)ival;
+            PUT_NARROW(data->long_);
+            PUT_WIDE  (data->long_);
+
+            data->ulong_.value_  = (unsigned long)ival;
+            PUT_NARROW(data->ulong_);
+            PUT_WIDE  (data->ulong_);
+
+#ifndef _RWSTD_NO_LONG_LONG
+
+            data->llong_.value_  = (_RWSTD_LONG_LONG)ival;
+            PUT_NARROW(data->llong_);
+            PUT_WIDE  (data->llong_);
+
+            data->ullong_.value_ =
+                (unsigned _RWSTD_LONG_LONG)ival;
+            PUT_NARROW(data->ullong_);
+            PUT_WIDE  (data->ullong_);
+
+#endif // _RWSTD_NO_LONG_LONG
+
+            data->dbl_.value_    = (double)ival;
+            PUT_NARROW(data->dbl_);
+            PUT_WIDE  (data->dbl_);
+
+#ifndef _RWSTD_NO_LONG_DOUBLE
+
+            data->ldbl_.value_   = (long double)ival;
+            PUT_NARROW(data->ldbl_);
+            PUT_WIDE  (data->ldbl_);
+
+#endif // _RWSTD_NO_LONG_DOUBLE
+
+            data->ptr_.value_    = (void*)ival;
+            PUT_NARROW(data->ptr_);
+            PUT_WIDE  (data->ptr_);
+
+            // test boolapha flag
+            if (ival & 1) fmt |= std::ios_base::boolalpha;
+
+            data->bool_.value_   = ival & 2 ? true : false;
+            PUT_NARROW(data->bool_);
+            PUT_WIDE  (data->bool_);
+
+            ++nlocales;
+        }
+        catch(...) {
+            // skip over bad locale
+            //rw_warn(0, 0, 0, "unable to create locale '%s'", name);
+        }
+
         if (nlocales == maxinx)
             break;
     }
@@ -290,6 +472,13 @@
              rw_opt_nloops, 1 != rw_opt_nloops,
              int (nlocales), "%#s", locales);
 
+    // avoid divide by zero in thread if there are no locales to test
+    if (nlocales < 1)
+    {
+        rw_fatal(nlocales != 0, __FILE__, __LINE__,
+            "failed to create one or more usable locales");
+    }
+
     rw_info (0, 0, 0, "exercising std::num_put<char>");
 
     test_char  = true;

Re: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
[...]
> Exactly. I'm okay with a warning. I'm considering adding a
> warning for each locale that couldn't be loaded. If no usable
> locale is available, I will issue another warning that the 'C'
> locale will be used.

You mean only when --locales= is used, right? I.e., not when
the test itself finds one that's bad. That sounds like useful
enhancement.

> 
>> The API is the same. The machinery is enabled automatically by
>> pointing the RWSTD_LOCALE_ROOT environment variable at the root
>> of the stdcxx locale database tree and referencing locales
>> installed under it by name (or pathname). Most of the locale
>> tests, including 22.locale.time.put.cpp, exercise both
>> implementations.
> 
> I'll add this to my todo list. Right now I think it is best if
> I get some commitable tests written. :)

Sounds good!
Martin

RE: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Travis Vitek <tv...@quovadx.com>.
 

Martin Sebor wrote:
>
>You mean when the user specifies --locales="foo bar" and one or
>more of them isn't available the test should give an error? Hmm,
>I agree that we should give some indication that one (or more)
>of the locales are unavailable in this case but think I would
>still prefer a warning to a hard error, simply because I see no
>harm in being robust (not every locale that /usr/bin/locale -a
>spits out is valid, so having the test weed out the bad ones
>can be useful when scripting things). Do you have a reason for
>wanting it to be an error?
>

Exactly. I'm okay with a warning. I'm considering adding a
warning for each locale that couldn't be loaded. If no usable
locale is available, I will issue another warning that the 'C'
locale will be used.

>
>The API is the same. The machinery is enabled automatically by
>pointing the RWSTD_LOCALE_ROOT environment variable at the root
>of the stdcxx locale database tree and referencing locales
>installed under it by name (or pathname). Most of the locale
>tests, including 22.locale.time.put.cpp, exercise both
>implementations.

I'll add this to my todo list. Right now I think it is best if
I get some commitable tests written. :)

>
>Martin
>

Re: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
>  
> 
> Martin Sebor wrote:
>> A test failure should indicate that there is a problem with 
>> the library,
>> not that the environment system is not equipped to exercise it.
> 
> Agreed.
> 
>> When
>> there's only one locale installed on a system (the "C" locale must
>> always be present) that's not a problem we can do much about. All it
>> means is that we can't tell if the library would work correctly if
>> there were more than one locale. So I think in this case the test
>> should just run all threads in the "C" locale. It won't be a very
>> useful test but it'll be better than nothing, don't you agree?
>>
> 
> The only issue I would have is when the test is run manually. There
> is a command line argument to explicitly specify the locales to use,
> and if none of them are available I think I'd expect the test to exit
> immediately with an error.

You mean when the user specifies --locales="foo bar" and one or
more of them isn't available the test should give an error? Hmm,
I agree that we should give some indication that one (or more)
of the locales are unavailable in this case but think I would
still prefer a warning to a hard error, simply because I see no
harm in being robust (not every locale that /usr/bin/locale -a
spits out is valid, so having the test weed out the bad ones
can be useful when scripting things). Do you have a reason for
wanting it to be an error?

> 
> I can live with defaulting to the C locale as long as there is a
> warning, which is already provided.
> 
>> Btw., the locale thread safety tests currently only exercise the
>> implementation when using the native (libc) locales. There's a big
>> chunk of code that deals with our own locales that's not being
>> tested. At some point we'll need to enhance the tests to exercise
>> this functionality as well.
>>
> 
> I'm not sure what needs to be added for this enhancement. Could you
> name a test or two that does this, or point me to the APIs I need
> to be using?

The API is the same. The machinery is enabled automatically by
pointing the RWSTD_LOCALE_ROOT environment variable at the root
of the stdcxx locale database tree and referencing locales
installed under it by name (or pathname). Most of the locale
tests, including 22.locale.time.put.cpp, exercise both
implementations.

Martin

RE: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Travis Vitek <tv...@quovadx.com>.
 

Martin Sebor wrote:
>
>A test failure should indicate that there is a problem with 
>the library,
>not that the environment system is not equipped to exercise it.

Agreed.

>When
>there's only one locale installed on a system (the "C" locale must
>always be present) that's not a problem we can do much about. All it
>means is that we can't tell if the library would work correctly if
>there were more than one locale. So I think in this case the test
>should just run all threads in the "C" locale. It won't be a very
>useful test but it'll be better than nothing, don't you agree?
>

The only issue I would have is when the test is run manually. There
is a command line argument to explicitly specify the locales to use,
and if none of them are available I think I'd expect the test to exit
immediately with an error.

I can live with defaulting to the C locale as long as there is a
warning, which is already provided.

>Btw., the locale thread safety tests currently only exercise the
>implementation when using the native (libc) locales. There's a big
>chunk of code that deals with our own locales that's not being
>tested. At some point we'll need to enhance the tests to exercise
>this functionality as well.
>

I'm not sure what needs to be added for this enhancement. Could you
name a test or two that does this, or point me to the APIs I need
to be using?

>Martin
>

Re: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
> 
[...]
>> [...]
>>> +    // avoid divide by zero in thread if there are no 
>> locales to test
>>> +    if (nlocales < 1) {
>>> +        rw_fatal(nlocales != 0, 0, __LINE__,
>>> +                 "failed to create one or more usable locales");
>> There are systems with no locales installed. I think we just want
>> rw_warn() here, not something as severe as rw_fatal(). (And we
>> want a space before the open paren :)
>>
> 
> Not exactly sure what you want here. I can put an rw_warn() followed
> immediately by a return, or I can have each thread test the number of
> locales again and bail early. If the threads are allowed to run, they
> will all fail with a div-by-zero when they try to mod by 'nlocales'.

A test failure should indicate that there is a problem with the library,
not that the environment system is not equipped to exercise it. When
there's only one locale installed on a system (the "C" locale must
always be present) that's not a problem we can do much about. All it
means is that we can't tell if the library would work correctly if
there were more than one locale. So I think in this case the test
should just run all threads in the "C" locale. It won't be a very
useful test but it'll be better than nothing, don't you agree?

Btw., the locale thread safety tests currently only exercise the
implementation when using the native (libc) locales. There's a big
chunk of code that deals with our own locales that's not being
tested. At some point we'll need to enhance the tests to exercise
this functionality as well.

Martin

RE: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Travis Vitek <tv...@quovadx.com>.

>Martin Sebor wrote:
>
>It looks like Outlook is even more braindead than I thought. It
>encoded the plain text attachment in base64, so there's no easy
>way to comment on it. Sigh. I had to copy the patch to an email
>and send it to myself to get the ticks ('>') so that my comments
>wouldn't get mixed up with the patch. Travis, you need to get
>yourself a decent mailer :) Anything but Outlook.
>

I'll apply the suggested changes, verify on gcc, and submit the patch
again using a mailer on the unix side.

>[...]
> > +    // avoid divide by zero in thread if there are no 
>locales to test
> > +    if (nlocales < 1) {
> > +        rw_fatal(nlocales != 0, 0, __LINE__,
> > +                 "failed to create one or more usable locales");
>
>There are systems with no locales installed. I think we just want
>rw_warn() here, not something as severe as rw_fatal(). (And we
>want a space before the open paren :)
>

Not exactly sure what you want here. I can put an rw_warn() followed
immediately by a return, or I can have each thread test the number of
locales again and bail early. If the threads are allowed to run, they
will all fail with a div-by-zero when they try to mod by 'nlocales'.

>Martin
>

Re: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
> Attached is a patch to enhance the time_put facet mt test. Threads
> verify that the values they put compare equal to those put in the
> primary thread.

It looks like Outlook is even more braindead than I thought. It
encoded the plain text attachment in base64, so there's no easy
way to comment on it. Sigh. I had to copy the patch to an email
and send it to myself to get the ticks ('>') so that my comments
wouldn't get mixed up with the patch. Travis, you need to get
yourself a decent mailer :) Anything but Outlook.

> 
> 2007-08-07	Travis Vitek	<vi...@roguewave.com>
> 
> 	* 22.locale.time.put.mt.cpp (MyIos, MyStreambuf, MyTimeData):
>       Added structures to simplify testing.
> 	(run_test): Build table of in/outputs for verification in test
>       threads. 
>       (thread_func): Assert that data written matches expected.
> 
> 

 > Index: 22.locale.time.put.mt.cpp
 > ===================================================================
 > --- 22.locale.time.put.mt.cpp    (revision 562577)
 > +++ 22.locale.time.put.mt.cpp    (working copy)
 > @@ -32,7 +32,7 @@
 >  #include <iterator>   // for ostreambuf_iterator
 >  #include <locale>     // for locale, time_put
 >
 > -#include <cstring>    // for strlen()
 > +#include <cstring>    // for strlen ()
 >  #include <ctime>      // for tm
 >
 >  #include <rw_locale.h>
 > @@ -60,76 +60,124 @@
 >  static std::size_t
 >  nlocales;
 >
 > 
-/**************************************************************************/ 

 >
 > +struct MyTimeData
 > +{
 > +    enum { BufferSize = 64 };
 >
 > -extern "C" {
 > +    // name of the locale the data corresponds to
 > +    const char* locale_name_;
 >
 > -bool test_char;    // exercise time_put<char>
 > -bool test_wchar;   // exercise time_put<wchar_t>
 > +    // optinally set to the named locale for threads to share
 > +    std::locale locale_;
 >
 > +    // the time struct used to generate strings below
 > +    std::tm time_;
 >
 > -static void*
 > -thread_func (void*)
 > +    // the format specifier
 > +    char format_;
 > +
 > +    // narrow representations of time_ given the
 > +    // locale_name_ and the format_
 > +    char ncs_ [BufferSize];
 > +    std::char_traits<char>::off_type nlen_;

If nlen_ is the length of ncs_ and can't be negative its type
should be size_t, not off_type (off_type is for file offsets).

[...]
 > +    charT* pubpptr () const {
 > +        return pptr ();

It's hard for me to tell but if the called function is defined
in a context that depends on a template parameter it needs to
be qualified with this-> (the compiler isn't allowed to look
in dependent bases when parsing unqualified calls). If you're
using MSVC to test your changes you might want to run them by
a recent version of gcc as well to catch more problems.

[...]
 > +static void*
 > +thread_func (void*)
 > +{
 > +    char                                       ncs
 > [MyTimeData::BufferSize];
 > +    MyIos<char, std::char_traits<char> >       nio;
 > +    MyStreambuf<char, std::char_traits<char> > nsb;
 > +
 > +#ifndef _RWSTD_NO_WCHAR_T
 > +    wchar_t                                          wcs
 > [MyTimeData::BufferSize];

Please make sure not to exceed 78 characters per line :)

[...]
 >          // save the name of the locale
 > -        const char* const locale_name = locales [i % nlocales];
 > +        const MyTimeData& data = my_time_data[i % nlocales];

Missing space before the open bracket.

 >
 >          // construct a named locale, get a reference to the time_put
 >          // facet from it and use it to format a random time value
 >          // using a random conversion specifier
 > -        const std::locale loc (locale_name);
 > +        const std::locale loc (data.locale_name_);
 >
 >          if (test_char) {
 >              // exercise the narrow char specialization of the facet
 >
 > +            // should this be hoisted out of the loop?
 >              const std::time_put<char> &tp =
 >                  std::use_facet<std::time_put<char> >(loc);

I think it probably should. In fact, creating the locale in the
thread function exercises (much) more than the tests are designed
to exercise. That's not without value but there should be a way
to reduce the scope of the test so as to avoid constructing the
same named locales in the thread function. 22.locale.ctype.mt.cpp
makes it possible under the --shared-locale command line option.

 >
[...]
 > +
 > +            const std::char_traits<char>::off_type nlen =
 > +                nsb.pubpptr() - nsb.pubpbase();

I understand why you declared nlen to have off_type but since
pbase() is required/guaranteed to be less than or equal to pptr()
the type of nlen should be size_t. That way there's no need to
worry about it being negative somewhere else.

 > +
 > +            RW_ASSERT (nlen == data.nlen_ &&
 > +                       !memcmp (data.ncs_, ncs, nlen));

memcmp() needs to be qualified with std:: here and operator&&
should be on the same line, like so:

+            RW_ASSERT (   nlen == data.nlen_
+                       && !memcmp (data.ncs_, ncs, nlen));

[...]
 > +        // fill in the time and results for this locale
 > +        MyTimeData& data = my_time_data[nlocales];

Missing space before the open bracket.

[...]
 > +#endif // _RWSTD_NO_WCHAR_T
 > +
 > +        } catch (...) {

No code should follow the closing brace.

[...]
 > +    // avoid divide by zero in thread if there are no locales to test
 > +    if (nlocales < 1) {
 > +        rw_fatal(nlocales != 0, 0, __LINE__,
 > +                 "failed to create one or more usable locales");

There are systems with no locales installed. I think we just want
rw_warn() here, not something as severe as rw_fatal(). (And we
want a space before the open paren :)

Martin

RE: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Travis Vitek <tv...@quovadx.com>.
Attached is a patch to enhance the time_put facet mt test. Threads
verify that the values they put compare equal to those put in the
primary thread.

2007-08-07	Travis Vitek	<vi...@roguewave.com>

	* 22.locale.time.put.mt.cpp (MyIos, MyStreambuf, MyTimeData):
      Added structures to simplify testing.
	(run_test): Build table of in/outputs for verification in test
      threads. 
      (thread_func): Assert that data written matches expected.


Re: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
> Looks like my attachment got stripped. This is really annoying. Does
> anyone have any idea why ezmlm hates me so?

I suspect it hates Outlook users more than the rest of us ;-)
Check to see the MIME type of the attachment in your outgoing
email and cross-reference it with the list in INFRA-1194 to
see if it's forbidden. If it is, you'll either need to figure
out how to change it to a type that's not forbidden or get
the Apache infrastructure team to allow that type. You might
also want to try sending it from a different mailer, such
as Mozilla or Thunderbird. They both work for me (for plain
text attachments.)

> 
> I'm going to try one more time. I'm attaching the patch file, and I'm
> pasting the contents below the patch information. If that fails I'll
> stop trying to paste/attach patches and I'll just start opening JIRAs.

That might be what we need to do in the meantime, until you
get one of the approaches above to work. It would be a huge
hassle to have to open a new Jira for every patch.

Martin

> 
> Travis
> 
> -----Original Message-----
> 
> Attached is a patch to enhance the time_put facet mt test. Threads
> verify that the values they put compare equal to those put in the
> primary thread.
> 
> 2007-08-07	Travis Vitek	<vi...@roguewave.com>
> 
> 	* 22.locale.time.put.mt.cpp (MyIos, MyStreambuf, MyTimeData):
>       Added structures to simplify testing.
> 	(run_test): Build table of in/outputs for verification in test
>       threads. 
>       (thread_func): Assert that data written matches expected.
> 
> Index: 22.locale.time.put.mt.cpp
> ===================================================================
> --- 22.locale.time.put.mt.cpp	(revision 562577)
> +++ 22.locale.time.put.mt.cpp	(working copy)
> @@ -32,7 +32,7 @@
>  #include <iterator>   // for ostreambuf_iterator
>  #include <locale>     // for locale, time_put
>  
> -#include <cstring>    // for strlen()
> +#include <cstring>    // for strlen ()
>  #include <ctime>      // for tm
>  
>  #include <rw_locale.h>
> @@ -60,76 +60,124 @@
>  static std::size_t
>  nlocales;
>  
> -/**********************************************************************
> ****/
> +struct MyTimeData
> +{
> +    enum { BufferSize = 64 };
>  
> -extern "C" {
> +    // name of the locale the data corresponds to
> +    const char* locale_name_;
>  
> -bool test_char;    // exercise time_put<char>
> -bool test_wchar;   // exercise time_put<wchar_t>
> +    // optinally set to the named locale for threads to share
> +    std::locale locale_;
>  
> +    // the time struct used to generate strings below
> +    std::tm time_;
>  
> -static void*
> -thread_func (void*)
> +    // the format specifier
> +    char format_;
> +
> +    // narrow representations of time_ given the 
> +    // locale_name_ and the format_
> +    char ncs_ [BufferSize];
> +    std::char_traits<char>::off_type nlen_;
> +    
> +#ifndef _RWSTD_NO_WCHAR_T
> +
> +    // wide representations of time_
> +    wchar_t wcs_ [BufferSize];
> +    std::char_traits<wchar_t>::off_type wlen_;
> +
> +#endif // _RWSTD_NO_WCHAR_T
> +
> +} my_time_data [MAX_THREADS];
> +
> +
> +/**********************************************************************
> ****/
> +
> +template <class charT, class Traits>
> +struct MyIos: std::basic_ios<charT, Traits>
>  {
> -    std::tm tmb = std::tm ();
> +    MyIos () {
> +        this->init (0);
> +    }
> +};
>  
> -    const char cvtspecs[] = "aAbBcCdDeFgGhHIjmMnprRStTuUVwWxXyYzZ%";
> +template <class charT, class Traits>
> +struct MyStreambuf: std::basic_streambuf<charT, Traits>
> +{
> +    typedef std::basic_streambuf<charT, Traits> Base;
>  
> -    // dummy streambuf-derived object the doesn't do anything
> -    // but allows ostreambuf_iterator to "think" it can write
> -    // to it
> -    struct NarrowBuf: std::streambuf {
> -        int_type overflow (int_type c) { return c; }
> -    } sb;
> +    MyStreambuf ()
> +        : Base () {
> +    }
>  
> -#ifndef _RWSTD_NO_WCHAR_T
> +    void pubsetp (charT *pbeg, charT *pend) {
> +        this->setp (pbeg, pend);
> +    }
>  
> -    struct WideBuf: std::wstreambuf {
> -        int_type overflow (int_type c) { return c; }
> -    } wsb;
> +    charT* pubpbase () const {
> +        return pbase ();
> +    }
>  
> -#endif   // _RWSTD_NO_WCHAR_T
> +    charT* pubpptr () const {
> +        return pptr ();
> +    }
>  
> -    struct Ios: std::ios {
> -        Ios () { this->init (0); }
> -    } io;
> +    charT* pubepptr () const {
> +        return epptr ();
> +    }
> +};
>  
> -    int j = 0;
> +#define countof(x) (sizeof (x) / sizeof (*x))
>  
> -    for (int i = 0; i != rw_opt_nloops; ++i, ++j) {
> +extern "C" {
>  
> -        // initialize tm with random but valid values
> -        tmb.tm_sec  = ++j % 61;
> -        tmb.tm_min  = ++j % 60;
> -        tmb.tm_min  = ++j % 60;
> -        tmb.tm_wday = ++j % 7;
> -        tmb.tm_mon  = ++j % 12;
> -        tmb.tm_year = ++j;
> +bool test_char;    // exercise time_put<char>
> +bool test_wchar;   // exercise time_put<wchar_t>
>  
> -        // generate a "random" conversion specifier from the set
> -        // of valid specifiers recognized by the facet to exercise
> -        // all (or most) code paths
> -        const char cvt = cvtspecs [i % (sizeof cvtspecs - 1)];
>  
> +static void*
> +thread_func (void*)
> +{
> +    char                                       ncs
> [MyTimeData::BufferSize];
> +    MyIos<char, std::char_traits<char> >       nio;
> +    MyStreambuf<char, std::char_traits<char> > nsb;
> +
> +#ifndef _RWSTD_NO_WCHAR_T
> +    wchar_t                                          wcs
> [MyTimeData::BufferSize];
> +    MyIos<wchar_t, std::char_traits<wchar_t> >       wio;
> +    MyStreambuf<wchar_t, std::char_traits<wchar_t> > wsb;
> +#endif // _RWSTD_NO_WCHAR_T
> +
> +    for (int i = 0; i != rw_opt_nloops; ++i) {
> +
>          // save the name of the locale
> -        const char* const locale_name = locales [i % nlocales];
> +        const MyTimeData& data = my_time_data[i % nlocales];
>  
>          // construct a named locale, get a reference to the time_put
>          // facet from it and use it to format a random time value
>          // using a random conversion specifier
> -        const std::locale loc (locale_name);
> +        const std::locale loc (data.locale_name_);
>  
>          if (test_char) {
>              // exercise the narrow char specialization of the facet
>  
> +            // should this be hoisted out of the loop?
>              const std::time_put<char> &tp =
>                  std::use_facet<std::time_put<char> >(loc);
>  
> -            // format a "random" but valid tm value using the random
> -            // format specifier
> -            tp.put (std::ostreambuf_iterator<char>(&sb),
> -                    io, ' ', &tmb, cvt);
> +            // assign data buffer to streambuf
> +            nsb.pubsetp (ncs, ncs + countof (ncs));
>  
> +            // format time using provided format specifier
> +            tp.put (std::ostreambuf_iterator<char>(&nsb),
> +                    nio, ' ', &data.time_, data.format_);
> +
> +            const std::char_traits<char>::off_type nlen =
> +                nsb.pubpptr() - nsb.pubpbase();
> +
> +            RW_ASSERT (nlen == data.nlen_ &&
> +                       !memcmp (data.ncs_, ncs, nlen));
>          }
>  
>          // both specializations may be tested at the same time
> @@ -139,14 +187,23 @@
>  
>  #ifndef _RWSTD_NO_WCHAR_T
>  
> +            // should this be hoisted out of the loop?
>              const std::time_put<wchar_t> &wtp =
>                  std::use_facet<std::time_put<wchar_t> >(loc);
>  
> +            wsb.pubsetp (wcs, wcs + countof (wcs));
> +
>              wtp.put (std::ostreambuf_iterator<wchar_t>(&wsb),
> -                     io, L' ', &tmb, cvt);
> +                     wio, L' ', &data.time_, data.format_);
>  
> -#endif   // _RWSTD_NO_WCHAR_T
> +            const std::char_traits<wchar_t>::off_type wlen =
> +                wsb.pubpptr() - wsb.pubpbase();
>  
> +            RW_ASSERT (wlen == data.wlen_ &&
> +                       !memcmp (data.wcs_, wcs, wlen));
> +
> +#endif // _RWSTD_NO_WCHAR_T
> +
>          }
>      }
>  
> @@ -160,19 +217,91 @@
>  static int
>  run_test (int, char**)
>  {
> -    // find all installed locales for which setlocale(LC_ALL) succeeds
> +    MyIos<char, std::char_traits<char> >       nio;
> +    MyStreambuf<char, std::char_traits<char> > nsb;
> +
> +#ifndef _RWSTD_NO_WCHAR_T
> +    MyIos<wchar_t, std::char_traits<wchar_t> >       wio;
> +    MyStreambuf<wchar_t, std::char_traits<wchar_t> > wsb;
> +#endif // _RWSTD_NO_WCHAR_T
> +
> +    // find all installed locales for which setlocale (LC_ALL) succeeds
>      const char* const locale_list =
>          rw_opt_locales ? rw_opt_locales : rw_locales (_RWSTD_LC_ALL);
>  
> -    const std::size_t maxinx = sizeof locales / sizeof *locales;
> +    const std::size_t maxinx = countof (locales);
>  
> -    for (const char *name = locale_list; *name; name += std::strlen
> (name) +1) {
> -        locales [nlocales++] = name;
> +    const char cvtspecs [] = "aAbBcCdDeFgGhHIjmMnprRStTuUVwWxXyYzZ%";
>  
> +    int j = 0;
> +    for (const char const *name = locale_list;
> +         *name;
> +         name += std::strlen (name) + 1) {
> +
> +        const std::size_t inx = nlocales;
> +        locales [inx] = name;
> +
> +        // fill in the time and results for this locale
> +        MyTimeData& data = my_time_data[nlocales];
> +        data.locale_name_ = name;
> +
> +        try {
> +            const std::locale loc (name);
> +
> +            // initialize tm with random but valid values
> +            data.time_.tm_sec  = ++j % 61;
> +            data.time_.tm_min  = ++j % 60;
> +            data.time_.tm_hour = ++j % 12;
> +            data.time_.tm_wday = ++j % 7;
> +            data.time_.tm_mon  = ++j % 12;
> +            data.time_.tm_mday = ++j % 31;
> +            data.time_.tm_yday = ++j % 366;
> +            data.time_.tm_year = ++j;
> +
> +            // get the "random" conversion specifier used to generate
> +            // the result string
> +            data.format_ = cvtspecs [nlocales % (sizeof cvtspecs - 1)];
> +
> +            const std::time_put<char> &np =
> +                std::use_facet<std::time_put<char> >(loc);
> +
> +            nsb.pubsetp (data.ncs_, data.ncs_ + countof (data.ncs_));
> +            
> +            np.put (std::ostreambuf_iterator<char>(&nsb),
> +                    nio, ' ', &data.time_, data.format_);
> +
> +            data.nlen_ = nsb.pubpptr() - nsb.pubpbase();
> +
> +#ifndef _RWSTD_NO_WCHAR_T
> +
> +            const std::time_put<wchar_t> &wp =
> +                std::use_facet<std::time_put<wchar_t> >(loc);
> +
> +            wsb.pubsetp (data.wcs_, data.wcs_ + countof (data.wcs_));
> +            
> +            wp.put (std::ostreambuf_iterator<wchar_t>(&wsb),
> +                    wio, L' ', &data.time_, data.format_);
> +
> +            data.wlen_ = wsb.pubpptr() - wsb.pubpbase();
> +
> +#endif // _RWSTD_NO_WCHAR_T
> +
> +        } catch (...) {
> +            // skip over bad locale
> +        }
> +
> +        nlocales += 1;
> +
>          if (nlocales == maxinx)
>              break;
>      }
>  
> +    // avoid divide by zero in thread if there are no locales to test
> +    if (nlocales < 1) {
> +        rw_fatal(nlocales != 0, 0, __LINE__,
> +                 "failed to create one or more usable locales");
> +    }
> +
>      rw_info (0, 0, 0,
>               "testing std::time_put<charT> with %d thread%{?}s%{;}, "
>               "%zu iteration%{?}s%{;} each, in locales { %{ .*A@} }",


RE: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Farid Zaripov <Fa...@epam.com>.
> -----Original Message-----
> From: Travis Vitek [mailto:tvitek@quovadx.com] 
> Sent: Tuesday, August 07, 2007 7:28 PM
> To: stdcxx-dev@incubator.apache.org
> Subject: RE: [PATCH] Update test 22.locale.time.put.mt.cpp to 
> validate results
> 
> 
> Looks like my attachment got stripped. This is really 
> annoying. Does anyone have any idea why ezmlm hates me so?
> 
> I'm going to try one more time. I'm attaching the patch file, 
> and I'm pasting the contents below the patch information. If 
> that fails I'll stop trying to paste/attach patches and I'll 
> just start opening JIRAs.

  Ezmlm strips any attachment from letters, produced by MS Outlook.
I don't know the reason, but it's a fact :( Personally, I'm using
the Mozilla Thunderbird to send letters with attachments, and
MS Outlook for any other letters.

Farid.

RE: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Travis Vitek <tv...@quovadx.com>.
Looks like my attachment got stripped. This is really annoying. Does
anyone have any idea why ezmlm hates me so?

I'm going to try one more time. I'm attaching the patch file, and I'm
pasting the contents below the patch information. If that fails I'll
stop trying to paste/attach patches and I'll just start opening JIRAs.

Travis

-----Original Message-----

Attached is a patch to enhance the time_put facet mt test. Threads
verify that the values they put compare equal to those put in the
primary thread.

2007-08-07	Travis Vitek	<vi...@roguewave.com>

	* 22.locale.time.put.mt.cpp (MyIos, MyStreambuf, MyTimeData):
      Added structures to simplify testing.
	(run_test): Build table of in/outputs for verification in test
      threads. 
      (thread_func): Assert that data written matches expected.

Index: 22.locale.time.put.mt.cpp
===================================================================
--- 22.locale.time.put.mt.cpp	(revision 562577)
+++ 22.locale.time.put.mt.cpp	(working copy)
@@ -32,7 +32,7 @@
 #include <iterator>   // for ostreambuf_iterator
 #include <locale>     // for locale, time_put
 
-#include <cstring>    // for strlen()
+#include <cstring>    // for strlen ()
 #include <ctime>      // for tm
 
 #include <rw_locale.h>
@@ -60,76 +60,124 @@
 static std::size_t
 nlocales;
 
-/**********************************************************************
****/
+struct MyTimeData
+{
+    enum { BufferSize = 64 };
 
-extern "C" {
+    // name of the locale the data corresponds to
+    const char* locale_name_;
 
-bool test_char;    // exercise time_put<char>
-bool test_wchar;   // exercise time_put<wchar_t>
+    // optinally set to the named locale for threads to share
+    std::locale locale_;
 
+    // the time struct used to generate strings below
+    std::tm time_;
 
-static void*
-thread_func (void*)
+    // the format specifier
+    char format_;
+
+    // narrow representations of time_ given the 
+    // locale_name_ and the format_
+    char ncs_ [BufferSize];
+    std::char_traits<char>::off_type nlen_;
+    
+#ifndef _RWSTD_NO_WCHAR_T
+
+    // wide representations of time_
+    wchar_t wcs_ [BufferSize];
+    std::char_traits<wchar_t>::off_type wlen_;
+
+#endif // _RWSTD_NO_WCHAR_T
+
+} my_time_data [MAX_THREADS];
+
+
+/**********************************************************************
****/
+
+template <class charT, class Traits>
+struct MyIos: std::basic_ios<charT, Traits>
 {
-    std::tm tmb = std::tm ();
+    MyIos () {
+        this->init (0);
+    }
+};
 
-    const char cvtspecs[] = "aAbBcCdDeFgGhHIjmMnprRStTuUVwWxXyYzZ%";
+template <class charT, class Traits>
+struct MyStreambuf: std::basic_streambuf<charT, Traits>
+{
+    typedef std::basic_streambuf<charT, Traits> Base;
 
-    // dummy streambuf-derived object the doesn't do anything
-    // but allows ostreambuf_iterator to "think" it can write
-    // to it
-    struct NarrowBuf: std::streambuf {
-        int_type overflow (int_type c) { return c; }
-    } sb;
+    MyStreambuf ()
+        : Base () {
+    }
 
-#ifndef _RWSTD_NO_WCHAR_T
+    void pubsetp (charT *pbeg, charT *pend) {
+        this->setp (pbeg, pend);
+    }
 
-    struct WideBuf: std::wstreambuf {
-        int_type overflow (int_type c) { return c; }
-    } wsb;
+    charT* pubpbase () const {
+        return pbase ();
+    }
 
-#endif   // _RWSTD_NO_WCHAR_T
+    charT* pubpptr () const {
+        return pptr ();
+    }
 
-    struct Ios: std::ios {
-        Ios () { this->init (0); }
-    } io;
+    charT* pubepptr () const {
+        return epptr ();
+    }
+};
 
-    int j = 0;
+#define countof(x) (sizeof (x) / sizeof (*x))
 
-    for (int i = 0; i != rw_opt_nloops; ++i, ++j) {
+extern "C" {
 
-        // initialize tm with random but valid values
-        tmb.tm_sec  = ++j % 61;
-        tmb.tm_min  = ++j % 60;
-        tmb.tm_min  = ++j % 60;
-        tmb.tm_wday = ++j % 7;
-        tmb.tm_mon  = ++j % 12;
-        tmb.tm_year = ++j;
+bool test_char;    // exercise time_put<char>
+bool test_wchar;   // exercise time_put<wchar_t>
 
-        // generate a "random" conversion specifier from the set
-        // of valid specifiers recognized by the facet to exercise
-        // all (or most) code paths
-        const char cvt = cvtspecs [i % (sizeof cvtspecs - 1)];
 
+static void*
+thread_func (void*)
+{
+    char                                       ncs
[MyTimeData::BufferSize];
+    MyIos<char, std::char_traits<char> >       nio;
+    MyStreambuf<char, std::char_traits<char> > nsb;
+
+#ifndef _RWSTD_NO_WCHAR_T
+    wchar_t                                          wcs
[MyTimeData::BufferSize];
+    MyIos<wchar_t, std::char_traits<wchar_t> >       wio;
+    MyStreambuf<wchar_t, std::char_traits<wchar_t> > wsb;
+#endif // _RWSTD_NO_WCHAR_T
+
+    for (int i = 0; i != rw_opt_nloops; ++i) {
+
         // save the name of the locale
-        const char* const locale_name = locales [i % nlocales];
+        const MyTimeData& data = my_time_data[i % nlocales];
 
         // construct a named locale, get a reference to the time_put
         // facet from it and use it to format a random time value
         // using a random conversion specifier
-        const std::locale loc (locale_name);
+        const std::locale loc (data.locale_name_);
 
         if (test_char) {
             // exercise the narrow char specialization of the facet
 
+            // should this be hoisted out of the loop?
             const std::time_put<char> &tp =
                 std::use_facet<std::time_put<char> >(loc);
 
-            // format a "random" but valid tm value using the random
-            // format specifier
-            tp.put (std::ostreambuf_iterator<char>(&sb),
-                    io, ' ', &tmb, cvt);
+            // assign data buffer to streambuf
+            nsb.pubsetp (ncs, ncs + countof (ncs));
 
+            // format time using provided format specifier
+            tp.put (std::ostreambuf_iterator<char>(&nsb),
+                    nio, ' ', &data.time_, data.format_);
+
+            const std::char_traits<char>::off_type nlen =
+                nsb.pubpptr() - nsb.pubpbase();
+
+            RW_ASSERT (nlen == data.nlen_ &&
+                       !memcmp (data.ncs_, ncs, nlen));
         }
 
         // both specializations may be tested at the same time
@@ -139,14 +187,23 @@
 
 #ifndef _RWSTD_NO_WCHAR_T
 
+            // should this be hoisted out of the loop?
             const std::time_put<wchar_t> &wtp =
                 std::use_facet<std::time_put<wchar_t> >(loc);
 
+            wsb.pubsetp (wcs, wcs + countof (wcs));
+
             wtp.put (std::ostreambuf_iterator<wchar_t>(&wsb),
-                     io, L' ', &tmb, cvt);
+                     wio, L' ', &data.time_, data.format_);
 
-#endif   // _RWSTD_NO_WCHAR_T
+            const std::char_traits<wchar_t>::off_type wlen =
+                wsb.pubpptr() - wsb.pubpbase();
 
+            RW_ASSERT (wlen == data.wlen_ &&
+                       !memcmp (data.wcs_, wcs, wlen));
+
+#endif // _RWSTD_NO_WCHAR_T
+
         }
     }
 
@@ -160,19 +217,91 @@
 static int
 run_test (int, char**)
 {
-    // find all installed locales for which setlocale(LC_ALL) succeeds
+    MyIos<char, std::char_traits<char> >       nio;
+    MyStreambuf<char, std::char_traits<char> > nsb;
+
+#ifndef _RWSTD_NO_WCHAR_T
+    MyIos<wchar_t, std::char_traits<wchar_t> >       wio;
+    MyStreambuf<wchar_t, std::char_traits<wchar_t> > wsb;
+#endif // _RWSTD_NO_WCHAR_T
+
+    // find all installed locales for which setlocale (LC_ALL) succeeds
     const char* const locale_list =
         rw_opt_locales ? rw_opt_locales : rw_locales (_RWSTD_LC_ALL);
 
-    const std::size_t maxinx = sizeof locales / sizeof *locales;
+    const std::size_t maxinx = countof (locales);
 
-    for (const char *name = locale_list; *name; name += std::strlen
(name) +1) {
-        locales [nlocales++] = name;
+    const char cvtspecs [] = "aAbBcCdDeFgGhHIjmMnprRStTuUVwWxXyYzZ%";
 
+    int j = 0;
+    for (const char const *name = locale_list;
+         *name;
+         name += std::strlen (name) + 1) {
+
+        const std::size_t inx = nlocales;
+        locales [inx] = name;
+
+        // fill in the time and results for this locale
+        MyTimeData& data = my_time_data[nlocales];
+        data.locale_name_ = name;
+
+        try {
+            const std::locale loc (name);
+
+            // initialize tm with random but valid values
+            data.time_.tm_sec  = ++j % 61;
+            data.time_.tm_min  = ++j % 60;
+            data.time_.tm_hour = ++j % 12;
+            data.time_.tm_wday = ++j % 7;
+            data.time_.tm_mon  = ++j % 12;
+            data.time_.tm_mday = ++j % 31;
+            data.time_.tm_yday = ++j % 366;
+            data.time_.tm_year = ++j;
+
+            // get the "random" conversion specifier used to generate
+            // the result string
+            data.format_ = cvtspecs [nlocales % (sizeof cvtspecs - 1)];
+
+            const std::time_put<char> &np =
+                std::use_facet<std::time_put<char> >(loc);
+
+            nsb.pubsetp (data.ncs_, data.ncs_ + countof (data.ncs_));
+            
+            np.put (std::ostreambuf_iterator<char>(&nsb),
+                    nio, ' ', &data.time_, data.format_);
+
+            data.nlen_ = nsb.pubpptr() - nsb.pubpbase();
+
+#ifndef _RWSTD_NO_WCHAR_T
+
+            const std::time_put<wchar_t> &wp =
+                std::use_facet<std::time_put<wchar_t> >(loc);
+
+            wsb.pubsetp (data.wcs_, data.wcs_ + countof (data.wcs_));
+            
+            wp.put (std::ostreambuf_iterator<wchar_t>(&wsb),
+                    wio, L' ', &data.time_, data.format_);
+
+            data.wlen_ = wsb.pubpptr() - wsb.pubpbase();
+
+#endif // _RWSTD_NO_WCHAR_T
+
+        } catch (...) {
+            // skip over bad locale
+        }
+
+        nlocales += 1;
+
         if (nlocales == maxinx)
             break;
     }
 
+    // avoid divide by zero in thread if there are no locales to test
+    if (nlocales < 1) {
+        rw_fatal(nlocales != 0, 0, __LINE__,
+                 "failed to create one or more usable locales");
+    }
+
     rw_info (0, 0, 0,
              "testing std::time_put<charT> with %d thread%{?}s%{;}, "
              "%zu iteration%{?}s%{;} each, in locales { %{ .*A@} }",

[PATCH] Update test 22.locale.time.put.mt.cpp to validate results

Posted by Travis Vitek <tv...@quovadx.com>.
Attached is a patch to enhance the time_put facet mt test. Threads
verify that the values they put compare equal to those put in the
primary thread.

2007-08-07	Travis Vitek	<vi...@roguewave.com>

	* 22.locale.time.put.mt.cpp (MyIos, MyStreambuf, MyTimeData):
      Added structures to simplify testing.
	(run_test): Build table of in/outputs for verification in test
      threads. 
      (thread_func): Assert that data written matches expected.


Re: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
>  
> 
> Martin Sebor wrote:
>> I'm afraid the patch didn't apply cleanly for me (probably due
>> to whitespace issues). Try attaching it to your post to see if
>> it makes it past ezmlm. If that doesn't work, unless you can
>> put it up on your web page somewgere you might need to open
>> a Jira for the enhancement and attach the patch to it.
>>
> 
> I'll create a new patch after making the recommended changes.
> 
>> A few comments on the changes follow inline...
>>
> 
> Most of the issues are with related to code formatting. I haven't really
> looked, but I haven't seen a code formatting style guide. Is there one?

Sort of. It's an aged document that probably needs to be dusted
off first before it's migrated from Rogue Wave to this site. Until
then, follow the style used in existing code :-) On the inside of
the Rogue Wave firewall you can also read the old document here:
http://iwww.cvo.roguewave.com/groups/qe/products/stdlib/styleguide.html

Martin

RE: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results

Posted by Travis Vitek <tv...@quovadx.com>.
 

Martin Sebor wrote:
>I'm afraid the patch didn't apply cleanly for me (probably due
>to whitespace issues). Try attaching it to your post to see if
>it makes it past ezmlm. If that doesn't work, unless you can
>put it up on your web page somewgere you might need to open
>a Jira for the enhancement and attach the patch to it.
>

I'll create a new patch after making the recommended changes.

>A few comments on the changes follow inline...
>

Most of the issues are with related to code formatting. I haven't really
looked, but I haven't seen a code formatting style guide. Is there one?

Travis

Re: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
>  
> 
>> -----Original Message-----
>> From: Martin Sebor [mailto:sebor@roguewave.com] 
>> Sent: Friday, August 10, 2007 1:42 PM
>> To: stdcxx-dev@incubator.apache.org
>> Subject: Re: [PATCH] Update test 22.locale.num.put.mt.cpp to 
>> validate results
>>
>> Travis Vitek wrote:
>>
>>>  
>>>
>>> Martin Sebor wrote:
>>>
>>>> We can't be using rw_assert() in a thread function (the driver
>>>> isn't thread safe yet, and if it was it would unnecessarily
>>>> synchronize the threads). Use the RW_ASSERT() macro instead.
>>>>
>>>
>>> I think there will be a lot of useful context that is lost if
>>> I switch to RW_ASSERT. It would be especially useful to know
>>> the active locale, the input value, and the output values in
>>> these test cases.
>> These values are invaluable when the tests fail due to a problem
>> other than a thread safety bug in the library but I'm not sure
>> they would be of much use in the case of a thread safety bug
>> that manifests itself by corrupting data. Do you have specific
>> scenario in mind where this information would be useful?
>>
>>
> 
> One example that I ran into in my limited testing was that a locale
> was getting created with the correct name, but it was not the correct
> locale that was getting used. The data wasn't corrupt, it was just
> for the wrong locale.
> 
>   # INFO (S1) (3 lines):
>   # TEXT: testing std::time_put<charT> with 4 threads, 100000 iterations
> each, in locales { "en_US" "es_MX" }
>   # CLAUSE: lib.locale.time.put
> 
>   # WARNING (S5) (3 lines):
>   # TEXT: locale="en_US" output="jue" expected="Thu"
>   # CLAUSE: lib.locale.time.put
> 
> Sure this is a threading bug, but IMO the generated message is more
> useful than the other option

I agree that it looks more informative, but I'm not sure it really
is. Could you reproduce this consistently from run to run? If not,
it might be just a coincidence.

In any event, I agree that if we can avoid the synchronization in
the successful cases providing additional detail in the diagnostic
messages can be helpful. But rather than complicating the test
driver API with this detail I'd make the driver (i.e., calls to
rw_assert() et al) thread-safe. It should be straightforward to
do: simply add a global mutex to the driver and lock in on entry
to each of these functions.

> 
[...]
> The other nice thing is that the test will continue, possibly exposing
> additional problems. 

I don't think we should trust the test after the first failure.
Also, since each thread function iterates thousands of times we
might end up with tens of thousands of lines of output. I don't
have a big problem with making this an option of the test just
so long as it's disabled by default (for nightly builds).

> 
[...]
> Yes, that would be another workable solution. Honestly I'm hoping that
> once these tests are up and running the bugs will be found and there
> won't be any problems to speak of. If that is the case, then all of this
> discussion is moot.

Well, of course :) Once all the bugs have been fixed there will
be no failures to report. But the value of the tests will not only
be in helping us find and fix the existing bugs but also in helping
us catch future regressions.

Martin

RE: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results

Posted by Travis Vitek <tv...@quovadx.com>.
 

>-----Original Message-----
>From: Martin Sebor [mailto:sebor@roguewave.com] 
>Sent: Friday, August 10, 2007 1:42 PM
>To: stdcxx-dev@incubator.apache.org
>Subject: Re: [PATCH] Update test 22.locale.num.put.mt.cpp to 
>validate results
>
>Travis Vitek wrote:
>
>>  
>> 
>> Martin Sebor wrote:
>> 
>>>We can't be using rw_assert() in a thread function (the driver
>>>isn't thread safe yet, and if it was it would unnecessarily
>>>synchronize the threads). Use the RW_ASSERT() macro instead.
>>>
>> 
>> 
>> I think there will be a lot of useful context that is lost if
>> I switch to RW_ASSERT. It would be especially useful to know
>> the active locale, the input value, and the output values in
>> these test cases.
>
>These values are invaluable when the tests fail due to a problem
>other than a thread safety bug in the library but I'm not sure
>they would be of much use in the case of a thread safety bug
>that manifests itself by corrupting data. Do you have specific
>scenario in mind where this information would be useful?
>
>

One example that I ran into in my limited testing was that a locale
was getting created with the correct name, but it was not the correct
locale that was getting used. The data wasn't corrupt, it was just
for the wrong locale.

  # INFO (S1) (3 lines):
  # TEXT: testing std::time_put<charT> with 4 threads, 100000 iterations
each, in locales { "en_US" "es_MX" }
  # CLAUSE: lib.locale.time.put

  # WARNING (S5) (3 lines):
  # TEXT: locale="en_US" output="jue" expected="Thu"
  # CLAUSE: lib.locale.time.put

Sure this is a threading bug, but IMO the generated message is more
useful than the other option

 
/amd/devco/vitek/stdcxx/trunk/tests/localization/22.locale.time.put.mt.c
pp:177: void* thread_func(void*): Assertion '!rw_strncmp(ncs,
data.ncs_)' failed.
  /nfs/devbuild/vitek/stdcxx/trunk/linux.gcc/lib/libstd15d.so[0xea7b16]
 
/nfs/devbuild/vitek/stdcxx/trunk/linux.gcc/lib/libstd15d.so(_ZN4__rw16__
rw_assert_failEPKcS1_iS1_+0x6e)[0xea7bae]
./22.locale.time.put.mt[0x804c08a]
  /lib/tls/libpthread.so.0[0x6c3371]
  /lib/tls/libc.so.6(__clone+0x5e)[0x52affe]
  Aborted

The other nice thing is that the test will continue, possibly exposing
additional problems. 

>> 
>> I'm thinking of doing something like this in each of the facet
>> tests.
>> 
>> _RW::__rw_synchronized __driver_syncronize;
>
>This would introduce a dependency of the test on the correct
>behavior of component that is being tested. To do something like
>this we'd need to introduce a separate synchronization mechanism
>into the test driver.

Agreed. Ugh.

>But I'm not sure that'll be necessary. I'd
>rather avoid any kind of synchronization in the thread function.
>If you do want to make this information available, I suggest
>returning it from the thread function to the main thread in the
>form of indices into the data array and having the main thread
>display it in a call to rw_fatal().

Yes, that would be another workable solution. Honestly I'm hoping that
once these tests are up and running the bugs will be found and there
won't be any problems to speak of. If that is the case, then all of this
discussion is moot.

Travis

Re: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:

>  
> 
> Martin Sebor wrote:
> 
>>We can't be using rw_assert() in a thread function (the driver
>>isn't thread safe yet, and if it was it would unnecessarily
>>synchronize the threads). Use the RW_ASSERT() macro instead.
>>
> 
> 
> I think there will be a lot of useful context that is lost if
> I switch to RW_ASSERT. It would be especially useful to know
> the active locale, the input value, and the output values in
> these test cases.

These values are invaluable when the tests fail due to a problem
other than a thread safety bug in the library but I'm not sure
they would be of much use in the case of a thread safety bug
that manifests itself by corrupting data. Do you have specific
scenario in mind where this information would be useful?

> 
> I'm thinking of doing something like this in each of the facet
> tests.
> 
> _RW::__rw_synchronized __driver_syncronize;

This would introduce a dependency of the test on the correct
behavior of component that is being tested. To do something like
this we'd need to introduce a separate synchronization mechanism
into the test driver. But I'm not sure that'll be necessary. I'd
rather avoid any kind of synchronization in the thread function.
If you do want to make this information available, I suggest
returning it from the thread function to the main thread in the
form of indices into the data array and having the main thread
display it in a call to rw_fatal(). Even so, I'd only make this
behavior available under an option and leave it disabled by
default. Once there's a thread safety bug, especially one that
corrupts data, the behavior of the whole program is undefined
and so we can't really trust anything, except the simplest
mechanisms such as assert()/abort().

Martin

> 
> #define rw_mt_safe_diag(fn, expr, args)                     \
>     if (!(expr)) {                                          \
>         __rw_guard guard (__driver_syncronize._C_guard ()); \
>         fn args;                                            \
>     }
> 
> In the test threads I would use it like this...
> 
>   rw_mt_safe_diag (rw_warn, !rw_strncmp(buf, expected),
>                    (0, 0, 0, "num_put<%s>::put(..., %s (%d)) "
>                     "did not match expected for locale(%#s). "
>                     "expected %#s but got %#s.",
>                     #charT, #valueT, val, loc, expect, buf));
> 
> I realize that this doesn't make the entire driver thread safe,
> but it does serialize access to everything that the threads are
> doing with the driver, and only when necessary.
> 
> What do you think?
> 
> Travis


Re: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results

Posted by Martin Sebor <se...@roguewave.com>.
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.

I'm afraid the patch didn't apply cleanly for me (probably due
to whitespace issues). Try attaching it to your post to see if
it makes it past ezmlm. If that doesn't work, unless you can
put it up on your web page somewgere you might need to open
a Jira for the enhancement and attach the patch to it.

A few comments on the changes follow inline...

> 
> 2007-08-06	Travis Vitek	<vi...@roguewave.com>
> 
> 	* 22.locale.num.put.mt.cpp: Added structure for MyIos,
> MyStreambuf
> 	and CnumData types to simplify testing.

This should be something like:

	* 22.locale.num.put.mt.cpp (MyIos, MyStreambuf, CnumData):
	Added structures to simplify testing.

[...]
> @@ -58,8 +60,98 @@
>  static std::size_t
>  nlocales;
>  
> +struct CnumData
> +{
> +    enum { BufferSize = 32 };
> +
> +    // name of the locale the data corresponds to
> +    const char* locale_name_;
> +
> +    // optinally set to the named locale for threads to share
> +    std::locale locale_;
> +
> +    // holder for original value and character representation
> +    template <typename T>

We've been discussing starting to rely on member templates in
the (near) future but we haven't decided to start yet. I suggest
to avoid using them until we've collectively decided it's okay
to do so. Here, you should be able to use a discriminated union
to get the same effect, or even simpler, use a double for all
data and convert it to the destination type as needed.

> +    struct result
> +    {
> +        result()
> +            : flags_(std::ios::dec)

We need a space before the open paren and the open curly goes on
the same line here (and in all nested definitions). The only place
we drop it is in namespace-scope definitions :)

> +        {
> +            memset(nrep_, 0, sizeof nrep_);
> +            memset(wrep_, 0, sizeof wrep_);

memset() must be qualified with std:: here (and followed by
a single space), although I don't think zeroing out the whole
buffer should really be necessary.

> +        }
> +
> +        T value_; // the original value
> +        std::ios::fmtflags flags_; // flags used

FWIW, justifying the data members to the same column it makes the
code more readable, like so:

+        T                  value_;   // the original value
+        std::ios::fmtflags flags_;   // flags used

> +
> +        // holds the narrow/wide character representation of value_
> +        char    nrep_[BufferSize]; 
> +        wchar_t wrep_[BufferSize];

Space missing before the open bracket.

[...]
> +    result<void*> ptr_;
> +
> +} num_put_data[MAX_THREADS];

Space missing before the open bracket.

[...]
> @@ -69,54 +161,43 @@
[...]
> -#ifndef _RWSTD_NO_WCHAR_T
> +// these macros should probably assert goodbit

Let's assert it if it's the right thing to do.

>  
> -    struct WideBuf: std::wstreambuf {
> -        int_type overflow (int_type c) { return c; }
> -    } wb;
> +#define TEST_PUT(cType, vType, field, value, fill, flag)            \

This is one big macro! It seems to me that it should only be
necessary to create all the objects (MyIos, MyStreambuf, etc.)
before entering the case/switch statement, and assert after
exiting it. Even better, it should be possible to create most
of them once per thread, and simply reset and reuse them in
each iteration. The only object that needs to be constructed
from scratch for every iteration is the iterator. Remember,
the loop needs to as tight and as efficient as possible in
order to maximize the contention in the tested function.

> +    {                                                               \
> +        MyIos<cType, std::char_traits<cType> > io;                  \
> +        io.imbue(loc); io.flags(flag); io.width(data->width_);      \
> +                                                                    \
> +        cType chars[CnumData::BufferSize];                          \
> +        memset(chars, 0, sizeof chars);                             \

memset() must be qualified with std:: here if you must zero out
all the elements. Alternatively, you can initialize the array
while defining it, like so:

+        cType chars [CnumData::BufferSize] = { cType () };

Btw., the name cType is misleading (there's a ctype facet). If
it refers to the same thing as charT I suggest calling it charT.

> +                                                                    \
> +        MyStreambuf<cType, std::char_traits<cType> > sb             \
> +           (chars, chars + CnumData::BufferSize);                   \
> +                                                                    \
> +        const std::ostreambuf_iterator<cType> iter (&sb);           \
> +                                                                    \
> +        np.put(iter, io, fill, value);                              \
> +        rw_assert(0 == memcmp(field, chars, sizeof chars),          \

We can't be using rw_assert() in a thread function (the driver
isn't thread safe yet, and if it was it would unnecessarily
synchronize the threads). Use the RW_ASSERT() macro instead.

[...]
>      for (int i = 0; i != rw_opt_nloops; ++i) {
>  
> -        // save the name of the locale
> -        const char* const locale_name = locales [i % nlocales];
> +        // fill in the value and results for this locale
> +        const CnumData* data = num_put_data + (i % nlocales);

If data isn't modified you should declare it const:

+ const CnumData* const data = num_put_data + (i % nlocales);

[...]
> @@ -131,75 +212,62 @@
[...]
> +                TEST_PUT_NARROW(bool, data->bool_);

Space missing before the open paren ab

> @@ -273,12 +343,124 @@
[...]
> +#if 1
> +#  define PUT_NARROW(it)                                           \
> +    PUT(char, it.nrep_, it.value_, ' ', it.flags_, np)
> +#else

Did you leave this block here on purpose?

[...]
> +        catch(...) {

Space missing before the open paren.

> +            // skip over bad locale
> +            //rw_warn(0, 0, 0, "unable to create locale '%s'", name);

I suggest you remove the commented out code just so someone doesn't
accidentally uncomment it (and cause failures due to rw_warn() not
being thread safe).

> +        }
> +
>          if (nlocales == maxinx)
>              break;
>      }
> @@ -290,6 +472,13 @@
>               rw_opt_nloops, 1 != rw_opt_nloops,
>               int (nlocales), "%#s", locales);
>  
> +    // avoid divide by zero in thread if there are no locales to test
> +    if (nlocales < 1)
> +    {

The paren goes on the same line as the if :)

> +        rw_fatal(nlocales != 0, __FILE__, __LINE__,
> +            "failed to create one or more usable locales");

The argument list continued on the second line should be aligned
with the argument list on the first line.

Martin