You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@stdcxx.apache.org by vi...@apache.org on 2008/03/28 22:28:42 UTC

svn commit: r642397 - in /stdcxx/trunk/tests/src: braceexp.cpp locale.cpp

Author: vitek
Date: Fri Mar 28 14:28:41 2008
New Revision: 642397

URL: http://svn.apache.org/viewvc?rev=642397&view=rev
Log:

2008-03-28  Travis Vitek  <vi...@roguewave.com>

	STDCXX-714
	* tests/src/braceexp.cpp: Remove _rw_isspace(), _rw_isupper() and
	_rw_islower(). Use appropriate C library calls instead.

	STDCXX-716
	* tests/src/locale.cpp: Remove _rw_isspace(), _rw_toupper() and
	_rw_tolower(). Use appropriate C library calls instead.
	

Modified:
    stdcxx/trunk/tests/src/braceexp.cpp
    stdcxx/trunk/tests/src/locale.cpp

Modified: stdcxx/trunk/tests/src/braceexp.cpp
URL: http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/braceexp.cpp?rev=642397&r1=642396&r2=642397&view=diff
==============================================================================
--- stdcxx/trunk/tests/src/braceexp.cpp (original)
+++ stdcxx/trunk/tests/src/braceexp.cpp Fri Mar 28 14:28:41 2008
@@ -3,55 +3,14 @@
 
 #include <stdlib.h> // for malloc(), free()
 #include <string.h> // for memcpy()
+#include <ctype.h>  // for isspace()
 #include <assert.h> // for assert()
 
 #include <rw_braceexp.h>
 
-inline int _rw_is_lower (int ch)
-{
-    switch (ch) {
-    case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
-    case 'g': case 'h': case 'i': case 'j': case 'k': case 'l':
-    case 'm': case 'n': case 'o': case 'p': case 'q': case 'r':
-    case 's': case 't': case 'u': case 'v': case 'w': case 'x':
-    case 'y': case 'z':
-        return 1;
-    }
-
-    return 0;
-}
-
-inline int _rw_is_upper (int ch)
-{
-    switch (ch) {
-    case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
-    case 'G': case 'H': case 'I': case 'J': case 'K': case 'L':
-    case 'M': case 'N': case 'O': case 'P': case 'Q': case 'R':
-    case 'S': case 'T': case 'U': case 'V': case 'W': case 'X':
-    case 'Y': case 'Z':
-        return 1;
-    }
-
-    return 0;
-}
-
-inline int _rw_is_space (int ch)
-{
-    switch (ch)
-    {
-    case '\n':
-    case '\r':
-    case '\t':
-    case ' ':
-        return 1;
-    }
-
-    return 0;
-}
-
 inline int _rw_is_not_space (int ch)
 {
-    return !_rw_is_space (ch);
+    return !isspace (ch);
 }
 
 // search `beg' to `end' for a character that `fn'
@@ -696,8 +655,8 @@
     char cend = beg [4];
 
     // only works if sequence characters are both lowercase or uppercase.
-    const int both_are_lower = _rw_is_lower (cbeg) && _rw_is_lower (cend);
-    const int both_are_upper = _rw_is_upper (cbeg) && _rw_is_upper (cend);
+    const int both_are_lower = islower (cbeg) && islower (cend);
+    const int both_are_upper = isupper (cbeg) && isupper (cend);
 
     if (! (both_are_lower || both_are_upper))
         return 0;
@@ -1050,7 +1009,7 @@
 
     while (tok_beg)
     {
-        const char* tok_end = _rw_find_match (tok_beg, end, _rw_is_space);
+        const char* tok_end = _rw_find_match (tok_beg, end, isspace);
         if (!tok_end)
             tok_end = end;
 

Modified: stdcxx/trunk/tests/src/locale.cpp
URL: http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/locale.cpp?rev=642397&r1=642396&r2=642397&view=diff
==============================================================================
--- stdcxx/trunk/tests/src/locale.cpp (original)
+++ stdcxx/trunk/tests/src/locale.cpp Fri Mar 28 14:28:41 2008
@@ -951,59 +951,6 @@
     struct _rw_locale_entry* next;
 };
 
-static int
-_rw_toupper (int chr)
-{
-    //if (chr < 'a' || 'z' < chr)
-    //    return chr;
-    //return chr - 'a' + 'A';
-    switch (chr)
-    {
-    case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
-    case 'g': case 'h': case 'i': case 'j': case 'k': case 'l':
-    case 'm': case 'n': case 'o': case 'p': case 'q': case 'r':
-    case 's': case 't': case 'u': case 'v': case 'w': case 'x':
-    case 'y': case 'z':
-        return chr - 'a' + 'A';
-    }
-
-    return chr;
-}
-
-static int
-_rw_tolower (int chr)
-{
-    //if (chr < 'A' || 'Z' < chr)
-    //    return chr;
-    //return chr - 'A' + 'a';
-    switch (chr)
-    {
-    case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
-    case 'G': case 'H': case 'I': case 'J': case 'K': case 'L':
-    case 'M': case 'N': case 'O': case 'P': case 'Q': case 'R':
-    case 'S': case 'T': case 'U': case 'V': case 'W': case 'X':
-    case 'Y': case 'Z':
-        return chr - 'A' + 'a';
-    }
-
-    return chr;
-}
-
-static int
-_rw_isspace (int chr)
-{
-    switch (chr)
-    {
-        case '\r':
-        case '\n':
-        case '\t':
-        case ' ':
-            return 1;
-    }
-
-    return 0;
-}
-
 struct _rw_locale_array {
     _rw_locale_entry* entries;
     _RWSTD_SIZE_T count;
@@ -1205,7 +1152,7 @@
             for (const char* charset = nl_langinfo (CODESET);
                  *charset;
                  ++charset) {
-                codeset [i++] = _rw_toupper (*charset);
+                codeset [i++] = toupper (*charset);
             }
 
             codeset [i] = '\0';
@@ -1225,7 +1172,7 @@
                 *encoding++ = '\0';
 
                 for (int n = 0; encoding [n]; ++n)
-                    encoding [n] = _rw_toupper (encoding [n]);
+                    encoding [n] = toupper (encoding [n]);
             }
 
             char* country = strrchr (locale, '_');
@@ -1233,13 +1180,13 @@
                 *country++ = '\0';
 
                 for (int n = 0; country [n]; ++n)
-                    country [n] = _rw_toupper (country [n]);
+                    country [n] = toupper (country [n]);
             }
             
             char* language = locale;
 
             for (int n = 0; language [n]; ++n)
-                language [n] = _rw_tolower (language [n]);
+                language [n] = tolower (language [n]);
 
             // use mapping databases to find the canonical
             // names for each part of the locale name
@@ -1296,7 +1243,7 @@
 
             // the canonical name for lookup
             sprintf (entry->canonical_name, "%s-%s-%d-%s",
-                     planguage, pcountry, MB_CUR_MAX, pencoding);
+                     planguage, pcountry, int (MB_CUR_MAX), pencoding);
 
             size += 1;
         }
@@ -1527,7 +1474,7 @@
 
             char* key = table_data + offset;
 
-            const int len = strcspn (key, "\r\n");
+            const size_t len = strcspn (key, "\r\n");
             key [len] = '\0';
 
             // skip the newline if it is there
@@ -1541,27 +1488,27 @@
             // make upper or lower case as requested
             if (upper_or_lower < 0) {
                 for (char* s = key; *s; ++s)
-                    *s = _rw_tolower (*s);
+                    *s = tolower (*s);
             }
             else if (0 < upper_or_lower) {
                 for (char* s = key; *s; ++s)
-                    *s = _rw_toupper (*s);
+                    *s = toupper (*s);
             }
 
             // if first character of new line is not whitespace, then we have a new
             // canonical name token
-            if (!_rw_isspace (*key)) {
+            if (!isspace (*key)) {
 
                 canonical_name = key;
 
                 // increment key past cannonical name
                 for (/**/; *key; ++key)
-                    if (_rw_isspace (*key))
+                    if (isspace (*key))
                         break;
             }
 
             // kill whitespace
-            while (_rw_isspace (*key))
+            while (isspace (*key))
                 *key++ = '\0';
 
             // key points to first non-whitespace after canonical name
@@ -1582,11 +1529,11 @@
                     *key++ = '\0';
 
                 // kill any whitespace before comma
-                for (char* bey = key - 1; _rw_isspace (*bey); --bey)
+                for (char* bey = key - 1; isspace (*bey); --bey)
                     *bey = '\0';
 
                 // kill whitespace after comma
-                while (_rw_isspace (*key))
+                while (isspace (*key))
                     *key++ = '\0';
 
                 // ensure we have enough entries



Re: svn commit: r642397 - in /stdcxx/trunk/tests/src: braceexp.cpp locale.cpp

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
>  
> 
>> Martin Sebor wrote:
>>
>> Great, that fixes locale.cpp. Thanks for doing that. I was also
>> (or mainly) pointing out the same problems in braceexp.cpp. I
>> hesitate to commit a fix myself in case you're working on the
>> file but here's a patch that addresses the remaining problems:
>>
> 
> Martin,
> 
> Your patch looks fine to me. Feel free to apply it.

I ended up checking in a slightly more extensive change to also
replace all occurrences of _RWSTD_SIZE_T with plain size_t for
readability.

http://svn.apache.org/viewvc?rev=644098&view=rev

Martin

RE: svn commit: r642397 - in /stdcxx/trunk/tests/src: braceexp.cpp locale.cpp

Posted by Travis Vitek <Tr...@roguewave.com>.
 

>Martin Sebor wrote:
>
>Great, that fixes locale.cpp. Thanks for doing that. I was also
>(or mainly) pointing out the same problems in braceexp.cpp. I
>hesitate to commit a fix myself in case you're working on the
>file but here's a patch that addresses the remaining problems:
>

Martin,

Your patch looks fine to me. Feel free to apply it.

Re: svn commit: r642397 - in /stdcxx/trunk/tests/src: braceexp.cpp locale.cpp

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
>  
> 
>> Martin Sebor wrote:
>>
>> PING?
>>
>> The sign extension issue is still there. I don't want it to 
>> get forgotten.
>>
>>
> 
> It was not forgotten. I decided to re-add the inline functions to avoid
> having to cast to unsigned char all over the place.
> 
>   http://svn.apache.org/viewvc?view=rev&revision=643214

Great, that fixes locale.cpp. Thanks for doing that. I was also
(or mainly) pointing out the same problems in braceexp.cpp. I
hesitate to commit a fix myself in case you're working on the
file but here's a patch that addresses the remaining problems:

Index: tests/src/braceexp.cpp
===================================================================
--- tests/src/braceexp.cpp      (revision 643552)
+++ tests/src/braceexp.cpp      (working copy)
@@ -35,6 +35,10 @@
  #include <rw_braceexp.h>


+// for convenience
+typedef unsigned char UChar;
+
+
  // search `beg' to `end' for a character that `fn'
  // returns non-zero.
  static const char*
@@ -46,7 +50,7 @@

      for (/**/; beg < end; ++beg) {

-        const bool is_space = 0 != isspace (*beg);
+        const bool is_space = 0 != isspace (UChar (*beg));

          if (!is_escaped && match_space == is_space) {
              return beg;
@@ -682,8 +686,10 @@
      char cend = beg [4];

      // only works if sequence characters are both lowercase or uppercase.
-    const int both_are_lower = islower (cbeg) && islower (cend);
-    const int both_are_upper = isupper (cbeg) && isupper (cend);
+    const int both_are_lower =
+        islower (UChar (cbeg)) && islower (UChar (cend));
+    const int both_are_upper =
+        isupper (UChar (cbeg)) && isupper (UChar (cend));

      if (! (both_are_lower || both_are_upper))
          return 0;

Martin

> 
> Travis
> 
>> Martin Sebor wrote:
>>> vitek@apache.org wrote:
>>>> Author: vitek
>>>> Date: Fri Mar 28 14:28:41 2008
>>>> New Revision: 642397
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=642397&view=rev
>>>> Log:
>>>>
>>>> 2008-03-28  Travis Vitek  <vi...@roguewave.com>
>>>>
>>>> 	STDCXX-714
>>>> 	* tests/src/braceexp.cpp: Remove _rw_isspace(), 
>> _rw_isupper() and
>>>> 	_rw_islower(). Use appropriate C library calls instead.
>>>>
>>>> 	STDCXX-716
>>>> 	* tests/src/locale.cpp: Remove _rw_isspace(), _rw_toupper() and
>>>> 	_rw_tolower(). Use appropriate C library calls instead.
>>>
>>> Travis, the isxxx() functions take an int argument and that their
>>> domain is the values between 0 and UCHAR_MAX plus EOF (and their
>>> behavior is undefined otherwise -- IIRC, the Microsoft functions
>>> abort in the undefined case). When passed a plain char, the
>>> argument is subject to sign extension where the type signed and
>>> values between CHAR_MIN and -1 are invalid. To make sure the
>>> functions always behave correctly it's best to cast the argument
>>> to unsigned char.
>>>
>>> Martin
>>>
>>>> 	
>>>>
>>>> Modified:
>>>>     stdcxx/trunk/tests/src/braceexp.cpp
>>>>     stdcxx/trunk/tests/src/locale.cpp
>>>>
>>>> Modified: stdcxx/trunk/tests/src/braceexp.cpp
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/braceexp.cp
>> p?rev=642397&r1=642396&r2=642397&view=diff
>> ===============================================================
>> ===============
>>>> --- stdcxx/trunk/tests/src/braceexp.cpp (original)
>>>> +++ stdcxx/trunk/tests/src/braceexp.cpp Fri Mar 28 14:28:41 2008
>>>> @@ -3,55 +3,14 @@
>>>>  
>>>>  #include <stdlib.h> // for malloc(), free()
>>>>  #include <string.h> // for memcpy()
>>>> +#include <ctype.h>  // for isspace()
>>>>  #include <assert.h> // for assert()
>>>>  
>>>>  #include <rw_braceexp.h>
>>>>  
>>>> -inline int _rw_is_lower (int ch)
>>>> -{
>>>> -    switch (ch) {
>>>> -    case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
>>>> -    case 'g': case 'h': case 'i': case 'j': case 'k': case 'l':
>>>> -    case 'm': case 'n': case 'o': case 'p': case 'q': case 'r':
>>>> -    case 's': case 't': case 'u': case 'v': case 'w': case 'x':
>>>> -    case 'y': case 'z':
>>>> -        return 1;
>>>> -    }
>>>> -
>>>> -    return 0;
>>>> -}
>>>> -
>>>> -inline int _rw_is_upper (int ch)
>>>> -{
>>>> -    switch (ch) {
>>>> -    case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
>>>> -    case 'G': case 'H': case 'I': case 'J': case 'K': case 'L':
>>>> -    case 'M': case 'N': case 'O': case 'P': case 'Q': case 'R':
>>>> -    case 'S': case 'T': case 'U': case 'V': case 'W': case 'X':
>>>> -    case 'Y': case 'Z':
>>>> -        return 1;
>>>> -    }
>>>> -
>>>> -    return 0;
>>>> -}
>>>> -
>>>> -inline int _rw_is_space (int ch)
>>>> -{
>>>> -    switch (ch)
>>>> -    {
>>>> -    case '\n':
>>>> -    case '\r':
>>>> -    case '\t':
>>>> -    case ' ':
>>>> -        return 1;
>>>> -    }
>>>> -
>>>> -    return 0;
>>>> -}
>>>> -
>>>>  inline int _rw_is_not_space (int ch)
>>>>  {
>>>> -    return !_rw_is_space (ch);
>>>> +    return !isspace (ch);
>>>>  }
>>>>  
>>>>  // search `beg' to `end' for a character that `fn'
>>>> @@ -696,8 +655,8 @@
>>>>      char cend = beg [4];
>>>>  
>>>>      // only works if sequence characters are both lowercase or
>>>> uppercase.
>>>> -    const int both_are_lower = _rw_is_lower (cbeg) && _rw_is_lower
>>>> (cend);
>>>> -    const int both_are_upper = _rw_is_upper (cbeg) && _rw_is_upper
>>>> (cend);
>>>> +    const int both_are_lower = islower (cbeg) && islower (cend);
>>>> +    const int both_are_upper = isupper (cbeg) && isupper (cend);
>>>>  
>>>>      if (! (both_are_lower || both_are_upper))
>>>>          return 0;
>>>> @@ -1050,7 +1009,7 @@
>>>>  
>>>>      while (tok_beg)
>>>>      {
>>>> -        const char* tok_end = _rw_find_match (tok_beg, end,
>>>> _rw_is_space);
>>>> +        const char* tok_end = _rw_find_match (tok_beg, 
>> end, isspace);
>>>>          if (!tok_end)
>>>>              tok_end = end;
>>>>  
>>>>
>>>> Modified: stdcxx/trunk/tests/src/locale.cpp
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/locale.cpp?
>> rev=642397&r1=642396&r2=642397&view=diff
>> ===============================================================
>> ===============
>>>> --- stdcxx/trunk/tests/src/locale.cpp (original)
>>>> +++ stdcxx/trunk/tests/src/locale.cpp Fri Mar 28 14:28:41 2008
>>>> @@ -951,59 +951,6 @@
>>>>      struct _rw_locale_entry* next;
>>>>  };
>>>>  
>>>> -static int
>>>> -_rw_toupper (int chr)
>>>> -{
>>>> -    //if (chr < 'a' || 'z' < chr)
>>>> -    //    return chr;
>>>> -    //return chr - 'a' + 'A';
>>>> -    switch (chr)
>>>> -    {
>>>> -    case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
>>>> -    case 'g': case 'h': case 'i': case 'j': case 'k': case 'l':
>>>> -    case 'm': case 'n': case 'o': case 'p': case 'q': case 'r':
>>>> -    case 's': case 't': case 'u': case 'v': case 'w': case 'x':
>>>> -    case 'y': case 'z':
>>>> -        return chr - 'a' + 'A';
>>>> -    }
>>>> -
>>>> -    return chr;
>>>> -}
>>>> -
>>>> -static int
>>>> -_rw_tolower (int chr)
>>>> -{
>>>> -    //if (chr < 'A' || 'Z' < chr)
>>>> -    //    return chr;
>>>> -    //return chr - 'A' + 'a';
>>>> -    switch (chr)
>>>> -    {
>>>> -    case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
>>>> -    case 'G': case 'H': case 'I': case 'J': case 'K': case 'L':
>>>> -    case 'M': case 'N': case 'O': case 'P': case 'Q': case 'R':
>>>> -    case 'S': case 'T': case 'U': case 'V': case 'W': case 'X':
>>>> -    case 'Y': case 'Z':
>>>> -        return chr - 'A' + 'a';
>>>> -    }
>>>> -
>>>> -    return chr;
>>>> -}
>>>> -
>>>> -static int
>>>> -_rw_isspace (int chr)
>>>> -{
>>>> -    switch (chr)
>>>> -    {
>>>> -        case '\r':
>>>> -        case '\n':
>>>> -        case '\t':
>>>> -        case ' ':
>>>> -            return 1;
>>>> -    }
>>>> -
>>>> -    return 0;
>>>> -}
>>>> -
>>>>  struct _rw_locale_array {
>>>>      _rw_locale_entry* entries;
>>>>      _RWSTD_SIZE_T count;
>>>> @@ -1205,7 +1152,7 @@
>>>>              for (const char* charset = nl_langinfo (CODESET);
>>>>                   *charset;
>>>>                   ++charset) {
>>>> -                codeset [i++] = _rw_toupper (*charset);
>>>> +                codeset [i++] = toupper (*charset);
>>>>              }
>>>>  
>>>>              codeset [i] = '\0';
>>>> @@ -1225,7 +1172,7 @@
>>>>                  *encoding++ = '\0';
>>>>  
>>>>                  for (int n = 0; encoding [n]; ++n)
>>>> -                    encoding [n] = _rw_toupper (encoding [n]);
>>>> +                    encoding [n] = toupper (encoding [n]);
>>>>              }
>>>>  
>>>>              char* country = strrchr (locale, '_');
>>>> @@ -1233,13 +1180,13 @@
>>>>                  *country++ = '\0';
>>>>  
>>>>                  for (int n = 0; country [n]; ++n)
>>>> -                    country [n] = _rw_toupper (country [n]);
>>>> +                    country [n] = toupper (country [n]);
>>>>              }
>>>>              
>>>>              char* language = locale;
>>>>  
>>>>              for (int n = 0; language [n]; ++n)
>>>> -                language [n] = _rw_tolower (language [n]);
>>>> +                language [n] = tolower (language [n]);
>>>>  
>>>>              // use mapping databases to find the canonical
>>>>              // names for each part of the locale name
>>>> @@ -1296,7 +1243,7 @@
>>>>  
>>>>              // the canonical name for lookup
>>>>              sprintf (entry->canonical_name, "%s-%s-%d-%s",
>>>> -                     planguage, pcountry, MB_CUR_MAX, pencoding);
>>>> +                     planguage, pcountry, int 
>> (MB_CUR_MAX), pencoding);
>>>>  
>>>>              size += 1;
>>>>          }
>>>> @@ -1527,7 +1474,7 @@
>>>>  
>>>>              char* key = table_data + offset;
>>>>  
>>>> -            const int len = strcspn (key, "\r\n");
>>>> +            const size_t len = strcspn (key, "\r\n");
>>>>              key [len] = '\0';
>>>>  
>>>>              // skip the newline if it is there
>>>> @@ -1541,27 +1488,27 @@
>>>>              // make upper or lower case as requested
>>>>              if (upper_or_lower < 0) {
>>>>                  for (char* s = key; *s; ++s)
>>>> -                    *s = _rw_tolower (*s);
>>>> +                    *s = tolower (*s);
>>>>              }
>>>>              else if (0 < upper_or_lower) {
>>>>                  for (char* s = key; *s; ++s)
>>>> -                    *s = _rw_toupper (*s);
>>>> +                    *s = toupper (*s);
>>>>              }
>>>>  
>>>>              // if first character of new line is not 
>> whitespace, then we
>>>> have a new
>>>>              // canonical name token
>>>> -            if (!_rw_isspace (*key)) {
>>>> +            if (!isspace (*key)) {
>>>>  
>>>>                  canonical_name = key;
>>>>  
>>>>                  // increment key past cannonical name
>>>>                  for (/**/; *key; ++key)
>>>> -                    if (_rw_isspace (*key))
>>>> +                    if (isspace (*key))
>>>>                          break;
>>>>              }
>>>>  
>>>>              // kill whitespace
>>>> -            while (_rw_isspace (*key))
>>>> +            while (isspace (*key))
>>>>                  *key++ = '\0';
>>>>  
>>>>              // key points to first non-whitespace after 
>> canonical name
>>>> @@ -1582,11 +1529,11 @@
>>>>                      *key++ = '\0';
>>>>  
>>>>                  // kill any whitespace before comma
>>>> -                for (char* bey = key - 1; _rw_isspace 
>> (*bey); --bey)
>>>> +                for (char* bey = key - 1; isspace (*bey); --bey)
>>>>                      *bey = '\0';
>>>>  
>>>>                  // kill whitespace after comma
>>>> -                while (_rw_isspace (*key))
>>>> +                while (isspace (*key))
>>>>                      *key++ = '\0';
>>>>  
>>>>                  // ensure we have enough entries
>>>>
>>>>
>>>>
>>>
>>>
>> -- 
>> View this message in context: 
>> http://www.nabble.com/Re%3A-svn-commit%3A-r642397---in--stdcxx-
>> trunk-tests-src%3A-braceexp.cpp-locale.cpp-tp16375460p16421931.html
>> Sent from the stdcxx-dev mailing list archive at Nabble.com.
>>
>>
> 


RE: svn commit: r642397 - in /stdcxx/trunk/tests/src: braceexp.cpp locale.cpp

Posted by Travis Vitek <Tr...@roguewave.com>.
 

>Martin Sebor wrote:
>
>PING?
>
>The sign extension issue is still there. I don't want it to 
>get forgotten.
>
>

It was not forgotten. I decided to re-add the inline functions to avoid
having to cast to unsigned char all over the place.

  http://svn.apache.org/viewvc?view=rev&revision=643214

Travis

>Martin Sebor wrote:
>> 
>> vitek@apache.org wrote:
>>> Author: vitek
>>> Date: Fri Mar 28 14:28:41 2008
>>> New Revision: 642397
>>> 
>>> URL: http://svn.apache.org/viewvc?rev=642397&view=rev
>>> Log:
>>> 
>>> 2008-03-28  Travis Vitek  <vi...@roguewave.com>
>>> 
>>> 	STDCXX-714
>>> 	* tests/src/braceexp.cpp: Remove _rw_isspace(), 
>_rw_isupper() and
>>> 	_rw_islower(). Use appropriate C library calls instead.
>>> 
>>> 	STDCXX-716
>>> 	* tests/src/locale.cpp: Remove _rw_isspace(), _rw_toupper() and
>>> 	_rw_tolower(). Use appropriate C library calls instead.
>> 
>> 
>> Travis, the isxxx() functions take an int argument and that their
>> domain is the values between 0 and UCHAR_MAX plus EOF (and their
>> behavior is undefined otherwise -- IIRC, the Microsoft functions
>> abort in the undefined case). When passed a plain char, the
>> argument is subject to sign extension where the type signed and
>> values between CHAR_MIN and -1 are invalid. To make sure the
>> functions always behave correctly it's best to cast the argument
>> to unsigned char.
>> 
>> Martin
>> 
>>> 	
>>> 
>>> Modified:
>>>     stdcxx/trunk/tests/src/braceexp.cpp
>>>     stdcxx/trunk/tests/src/locale.cpp
>>> 
>>> Modified: stdcxx/trunk/tests/src/braceexp.cpp
>>> URL:
>>> 
>http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/braceexp.cp
>p?rev=642397&r1=642396&r2=642397&view=diff
>>> 
>===============================================================
>===============
>>> --- stdcxx/trunk/tests/src/braceexp.cpp (original)
>>> +++ stdcxx/trunk/tests/src/braceexp.cpp Fri Mar 28 14:28:41 2008
>>> @@ -3,55 +3,14 @@
>>>  
>>>  #include <stdlib.h> // for malloc(), free()
>>>  #include <string.h> // for memcpy()
>>> +#include <ctype.h>  // for isspace()
>>>  #include <assert.h> // for assert()
>>>  
>>>  #include <rw_braceexp.h>
>>>  
>>> -inline int _rw_is_lower (int ch)
>>> -{
>>> -    switch (ch) {
>>> -    case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
>>> -    case 'g': case 'h': case 'i': case 'j': case 'k': case 'l':
>>> -    case 'm': case 'n': case 'o': case 'p': case 'q': case 'r':
>>> -    case 's': case 't': case 'u': case 'v': case 'w': case 'x':
>>> -    case 'y': case 'z':
>>> -        return 1;
>>> -    }
>>> -
>>> -    return 0;
>>> -}
>>> -
>>> -inline int _rw_is_upper (int ch)
>>> -{
>>> -    switch (ch) {
>>> -    case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
>>> -    case 'G': case 'H': case 'I': case 'J': case 'K': case 'L':
>>> -    case 'M': case 'N': case 'O': case 'P': case 'Q': case 'R':
>>> -    case 'S': case 'T': case 'U': case 'V': case 'W': case 'X':
>>> -    case 'Y': case 'Z':
>>> -        return 1;
>>> -    }
>>> -
>>> -    return 0;
>>> -}
>>> -
>>> -inline int _rw_is_space (int ch)
>>> -{
>>> -    switch (ch)
>>> -    {
>>> -    case '\n':
>>> -    case '\r':
>>> -    case '\t':
>>> -    case ' ':
>>> -        return 1;
>>> -    }
>>> -
>>> -    return 0;
>>> -}
>>> -
>>>  inline int _rw_is_not_space (int ch)
>>>  {
>>> -    return !_rw_is_space (ch);
>>> +    return !isspace (ch);
>>>  }
>>>  
>>>  // search `beg' to `end' for a character that `fn'
>>> @@ -696,8 +655,8 @@
>>>      char cend = beg [4];
>>>  
>>>      // only works if sequence characters are both lowercase or
>>> uppercase.
>>> -    const int both_are_lower = _rw_is_lower (cbeg) && _rw_is_lower
>>> (cend);
>>> -    const int both_are_upper = _rw_is_upper (cbeg) && _rw_is_upper
>>> (cend);
>>> +    const int both_are_lower = islower (cbeg) && islower (cend);
>>> +    const int both_are_upper = isupper (cbeg) && isupper (cend);
>>>  
>>>      if (! (both_are_lower || both_are_upper))
>>>          return 0;
>>> @@ -1050,7 +1009,7 @@
>>>  
>>>      while (tok_beg)
>>>      {
>>> -        const char* tok_end = _rw_find_match (tok_beg, end,
>>> _rw_is_space);
>>> +        const char* tok_end = _rw_find_match (tok_beg, 
>end, isspace);
>>>          if (!tok_end)
>>>              tok_end = end;
>>>  
>>> 
>>> Modified: stdcxx/trunk/tests/src/locale.cpp
>>> URL:
>>> 
>http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/locale.cpp?
>rev=642397&r1=642396&r2=642397&view=diff
>>> 
>===============================================================
>===============
>>> --- stdcxx/trunk/tests/src/locale.cpp (original)
>>> +++ stdcxx/trunk/tests/src/locale.cpp Fri Mar 28 14:28:41 2008
>>> @@ -951,59 +951,6 @@
>>>      struct _rw_locale_entry* next;
>>>  };
>>>  
>>> -static int
>>> -_rw_toupper (int chr)
>>> -{
>>> -    //if (chr < 'a' || 'z' < chr)
>>> -    //    return chr;
>>> -    //return chr - 'a' + 'A';
>>> -    switch (chr)
>>> -    {
>>> -    case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
>>> -    case 'g': case 'h': case 'i': case 'j': case 'k': case 'l':
>>> -    case 'm': case 'n': case 'o': case 'p': case 'q': case 'r':
>>> -    case 's': case 't': case 'u': case 'v': case 'w': case 'x':
>>> -    case 'y': case 'z':
>>> -        return chr - 'a' + 'A';
>>> -    }
>>> -
>>> -    return chr;
>>> -}
>>> -
>>> -static int
>>> -_rw_tolower (int chr)
>>> -{
>>> -    //if (chr < 'A' || 'Z' < chr)
>>> -    //    return chr;
>>> -    //return chr - 'A' + 'a';
>>> -    switch (chr)
>>> -    {
>>> -    case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
>>> -    case 'G': case 'H': case 'I': case 'J': case 'K': case 'L':
>>> -    case 'M': case 'N': case 'O': case 'P': case 'Q': case 'R':
>>> -    case 'S': case 'T': case 'U': case 'V': case 'W': case 'X':
>>> -    case 'Y': case 'Z':
>>> -        return chr - 'A' + 'a';
>>> -    }
>>> -
>>> -    return chr;
>>> -}
>>> -
>>> -static int
>>> -_rw_isspace (int chr)
>>> -{
>>> -    switch (chr)
>>> -    {
>>> -        case '\r':
>>> -        case '\n':
>>> -        case '\t':
>>> -        case ' ':
>>> -            return 1;
>>> -    }
>>> -
>>> -    return 0;
>>> -}
>>> -
>>>  struct _rw_locale_array {
>>>      _rw_locale_entry* entries;
>>>      _RWSTD_SIZE_T count;
>>> @@ -1205,7 +1152,7 @@
>>>              for (const char* charset = nl_langinfo (CODESET);
>>>                   *charset;
>>>                   ++charset) {
>>> -                codeset [i++] = _rw_toupper (*charset);
>>> +                codeset [i++] = toupper (*charset);
>>>              }
>>>  
>>>              codeset [i] = '\0';
>>> @@ -1225,7 +1172,7 @@
>>>                  *encoding++ = '\0';
>>>  
>>>                  for (int n = 0; encoding [n]; ++n)
>>> -                    encoding [n] = _rw_toupper (encoding [n]);
>>> +                    encoding [n] = toupper (encoding [n]);
>>>              }
>>>  
>>>              char* country = strrchr (locale, '_');
>>> @@ -1233,13 +1180,13 @@
>>>                  *country++ = '\0';
>>>  
>>>                  for (int n = 0; country [n]; ++n)
>>> -                    country [n] = _rw_toupper (country [n]);
>>> +                    country [n] = toupper (country [n]);
>>>              }
>>>              
>>>              char* language = locale;
>>>  
>>>              for (int n = 0; language [n]; ++n)
>>> -                language [n] = _rw_tolower (language [n]);
>>> +                language [n] = tolower (language [n]);
>>>  
>>>              // use mapping databases to find the canonical
>>>              // names for each part of the locale name
>>> @@ -1296,7 +1243,7 @@
>>>  
>>>              // the canonical name for lookup
>>>              sprintf (entry->canonical_name, "%s-%s-%d-%s",
>>> -                     planguage, pcountry, MB_CUR_MAX, pencoding);
>>> +                     planguage, pcountry, int 
>(MB_CUR_MAX), pencoding);
>>>  
>>>              size += 1;
>>>          }
>>> @@ -1527,7 +1474,7 @@
>>>  
>>>              char* key = table_data + offset;
>>>  
>>> -            const int len = strcspn (key, "\r\n");
>>> +            const size_t len = strcspn (key, "\r\n");
>>>              key [len] = '\0';
>>>  
>>>              // skip the newline if it is there
>>> @@ -1541,27 +1488,27 @@
>>>              // make upper or lower case as requested
>>>              if (upper_or_lower < 0) {
>>>                  for (char* s = key; *s; ++s)
>>> -                    *s = _rw_tolower (*s);
>>> +                    *s = tolower (*s);
>>>              }
>>>              else if (0 < upper_or_lower) {
>>>                  for (char* s = key; *s; ++s)
>>> -                    *s = _rw_toupper (*s);
>>> +                    *s = toupper (*s);
>>>              }
>>>  
>>>              // if first character of new line is not 
>whitespace, then we
>>> have a new
>>>              // canonical name token
>>> -            if (!_rw_isspace (*key)) {
>>> +            if (!isspace (*key)) {
>>>  
>>>                  canonical_name = key;
>>>  
>>>                  // increment key past cannonical name
>>>                  for (/**/; *key; ++key)
>>> -                    if (_rw_isspace (*key))
>>> +                    if (isspace (*key))
>>>                          break;
>>>              }
>>>  
>>>              // kill whitespace
>>> -            while (_rw_isspace (*key))
>>> +            while (isspace (*key))
>>>                  *key++ = '\0';
>>>  
>>>              // key points to first non-whitespace after 
>canonical name
>>> @@ -1582,11 +1529,11 @@
>>>                      *key++ = '\0';
>>>  
>>>                  // kill any whitespace before comma
>>> -                for (char* bey = key - 1; _rw_isspace 
>(*bey); --bey)
>>> +                for (char* bey = key - 1; isspace (*bey); --bey)
>>>                      *bey = '\0';
>>>  
>>>                  // kill whitespace after comma
>>> -                while (_rw_isspace (*key))
>>> +                while (isspace (*key))
>>>                      *key++ = '\0';
>>>  
>>>                  // ensure we have enough entries
>>> 
>>> 
>>> 
>> 
>> 
>> 
>
>-- 
>View this message in context: 
>http://www.nabble.com/Re%3A-svn-commit%3A-r642397---in--stdcxx-
>trunk-tests-src%3A-braceexp.cpp-locale.cpp-tp16375460p16421931.html
>Sent from the stdcxx-dev mailing list archive at Nabble.com.
>
>

Re: svn commit: r642397 - in /stdcxx/trunk/tests/src: braceexp.cpp locale.cpp

Posted by Martin Sebor <se...@roguewave.com>.
PING?

The sign extension issue is still there. I don't want it to get forgotten.


Martin Sebor wrote:
> 
> vitek@apache.org wrote:
>> Author: vitek
>> Date: Fri Mar 28 14:28:41 2008
>> New Revision: 642397
>> 
>> URL: http://svn.apache.org/viewvc?rev=642397&view=rev
>> Log:
>> 
>> 2008-03-28  Travis Vitek  <vi...@roguewave.com>
>> 
>> 	STDCXX-714
>> 	* tests/src/braceexp.cpp: Remove _rw_isspace(), _rw_isupper() and
>> 	_rw_islower(). Use appropriate C library calls instead.
>> 
>> 	STDCXX-716
>> 	* tests/src/locale.cpp: Remove _rw_isspace(), _rw_toupper() and
>> 	_rw_tolower(). Use appropriate C library calls instead.
> 
> 
> Travis, the isxxx() functions take an int argument and that their
> domain is the values between 0 and UCHAR_MAX plus EOF (and their
> behavior is undefined otherwise -- IIRC, the Microsoft functions
> abort in the undefined case). When passed a plain char, the
> argument is subject to sign extension where the type signed and
> values between CHAR_MIN and -1 are invalid. To make sure the
> functions always behave correctly it's best to cast the argument
> to unsigned char.
> 
> Martin
> 
>> 	
>> 
>> Modified:
>>     stdcxx/trunk/tests/src/braceexp.cpp
>>     stdcxx/trunk/tests/src/locale.cpp
>> 
>> Modified: stdcxx/trunk/tests/src/braceexp.cpp
>> URL:
>> http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/braceexp.cpp?rev=642397&r1=642396&r2=642397&view=diff
>> ==============================================================================
>> --- stdcxx/trunk/tests/src/braceexp.cpp (original)
>> +++ stdcxx/trunk/tests/src/braceexp.cpp Fri Mar 28 14:28:41 2008
>> @@ -3,55 +3,14 @@
>>  
>>  #include <stdlib.h> // for malloc(), free()
>>  #include <string.h> // for memcpy()
>> +#include <ctype.h>  // for isspace()
>>  #include <assert.h> // for assert()
>>  
>>  #include <rw_braceexp.h>
>>  
>> -inline int _rw_is_lower (int ch)
>> -{
>> -    switch (ch) {
>> -    case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
>> -    case 'g': case 'h': case 'i': case 'j': case 'k': case 'l':
>> -    case 'm': case 'n': case 'o': case 'p': case 'q': case 'r':
>> -    case 's': case 't': case 'u': case 'v': case 'w': case 'x':
>> -    case 'y': case 'z':
>> -        return 1;
>> -    }
>> -
>> -    return 0;
>> -}
>> -
>> -inline int _rw_is_upper (int ch)
>> -{
>> -    switch (ch) {
>> -    case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
>> -    case 'G': case 'H': case 'I': case 'J': case 'K': case 'L':
>> -    case 'M': case 'N': case 'O': case 'P': case 'Q': case 'R':
>> -    case 'S': case 'T': case 'U': case 'V': case 'W': case 'X':
>> -    case 'Y': case 'Z':
>> -        return 1;
>> -    }
>> -
>> -    return 0;
>> -}
>> -
>> -inline int _rw_is_space (int ch)
>> -{
>> -    switch (ch)
>> -    {
>> -    case '\n':
>> -    case '\r':
>> -    case '\t':
>> -    case ' ':
>> -        return 1;
>> -    }
>> -
>> -    return 0;
>> -}
>> -
>>  inline int _rw_is_not_space (int ch)
>>  {
>> -    return !_rw_is_space (ch);
>> +    return !isspace (ch);
>>  }
>>  
>>  // search `beg' to `end' for a character that `fn'
>> @@ -696,8 +655,8 @@
>>      char cend = beg [4];
>>  
>>      // only works if sequence characters are both lowercase or
>> uppercase.
>> -    const int both_are_lower = _rw_is_lower (cbeg) && _rw_is_lower
>> (cend);
>> -    const int both_are_upper = _rw_is_upper (cbeg) && _rw_is_upper
>> (cend);
>> +    const int both_are_lower = islower (cbeg) && islower (cend);
>> +    const int both_are_upper = isupper (cbeg) && isupper (cend);
>>  
>>      if (! (both_are_lower || both_are_upper))
>>          return 0;
>> @@ -1050,7 +1009,7 @@
>>  
>>      while (tok_beg)
>>      {
>> -        const char* tok_end = _rw_find_match (tok_beg, end,
>> _rw_is_space);
>> +        const char* tok_end = _rw_find_match (tok_beg, end, isspace);
>>          if (!tok_end)
>>              tok_end = end;
>>  
>> 
>> Modified: stdcxx/trunk/tests/src/locale.cpp
>> URL:
>> http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/locale.cpp?rev=642397&r1=642396&r2=642397&view=diff
>> ==============================================================================
>> --- stdcxx/trunk/tests/src/locale.cpp (original)
>> +++ stdcxx/trunk/tests/src/locale.cpp Fri Mar 28 14:28:41 2008
>> @@ -951,59 +951,6 @@
>>      struct _rw_locale_entry* next;
>>  };
>>  
>> -static int
>> -_rw_toupper (int chr)
>> -{
>> -    //if (chr < 'a' || 'z' < chr)
>> -    //    return chr;
>> -    //return chr - 'a' + 'A';
>> -    switch (chr)
>> -    {
>> -    case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
>> -    case 'g': case 'h': case 'i': case 'j': case 'k': case 'l':
>> -    case 'm': case 'n': case 'o': case 'p': case 'q': case 'r':
>> -    case 's': case 't': case 'u': case 'v': case 'w': case 'x':
>> -    case 'y': case 'z':
>> -        return chr - 'a' + 'A';
>> -    }
>> -
>> -    return chr;
>> -}
>> -
>> -static int
>> -_rw_tolower (int chr)
>> -{
>> -    //if (chr < 'A' || 'Z' < chr)
>> -    //    return chr;
>> -    //return chr - 'A' + 'a';
>> -    switch (chr)
>> -    {
>> -    case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
>> -    case 'G': case 'H': case 'I': case 'J': case 'K': case 'L':
>> -    case 'M': case 'N': case 'O': case 'P': case 'Q': case 'R':
>> -    case 'S': case 'T': case 'U': case 'V': case 'W': case 'X':
>> -    case 'Y': case 'Z':
>> -        return chr - 'A' + 'a';
>> -    }
>> -
>> -    return chr;
>> -}
>> -
>> -static int
>> -_rw_isspace (int chr)
>> -{
>> -    switch (chr)
>> -    {
>> -        case '\r':
>> -        case '\n':
>> -        case '\t':
>> -        case ' ':
>> -            return 1;
>> -    }
>> -
>> -    return 0;
>> -}
>> -
>>  struct _rw_locale_array {
>>      _rw_locale_entry* entries;
>>      _RWSTD_SIZE_T count;
>> @@ -1205,7 +1152,7 @@
>>              for (const char* charset = nl_langinfo (CODESET);
>>                   *charset;
>>                   ++charset) {
>> -                codeset [i++] = _rw_toupper (*charset);
>> +                codeset [i++] = toupper (*charset);
>>              }
>>  
>>              codeset [i] = '\0';
>> @@ -1225,7 +1172,7 @@
>>                  *encoding++ = '\0';
>>  
>>                  for (int n = 0; encoding [n]; ++n)
>> -                    encoding [n] = _rw_toupper (encoding [n]);
>> +                    encoding [n] = toupper (encoding [n]);
>>              }
>>  
>>              char* country = strrchr (locale, '_');
>> @@ -1233,13 +1180,13 @@
>>                  *country++ = '\0';
>>  
>>                  for (int n = 0; country [n]; ++n)
>> -                    country [n] = _rw_toupper (country [n]);
>> +                    country [n] = toupper (country [n]);
>>              }
>>              
>>              char* language = locale;
>>  
>>              for (int n = 0; language [n]; ++n)
>> -                language [n] = _rw_tolower (language [n]);
>> +                language [n] = tolower (language [n]);
>>  
>>              // use mapping databases to find the canonical
>>              // names for each part of the locale name
>> @@ -1296,7 +1243,7 @@
>>  
>>              // the canonical name for lookup
>>              sprintf (entry->canonical_name, "%s-%s-%d-%s",
>> -                     planguage, pcountry, MB_CUR_MAX, pencoding);
>> +                     planguage, pcountry, int (MB_CUR_MAX), pencoding);
>>  
>>              size += 1;
>>          }
>> @@ -1527,7 +1474,7 @@
>>  
>>              char* key = table_data + offset;
>>  
>> -            const int len = strcspn (key, "\r\n");
>> +            const size_t len = strcspn (key, "\r\n");
>>              key [len] = '\0';
>>  
>>              // skip the newline if it is there
>> @@ -1541,27 +1488,27 @@
>>              // make upper or lower case as requested
>>              if (upper_or_lower < 0) {
>>                  for (char* s = key; *s; ++s)
>> -                    *s = _rw_tolower (*s);
>> +                    *s = tolower (*s);
>>              }
>>              else if (0 < upper_or_lower) {
>>                  for (char* s = key; *s; ++s)
>> -                    *s = _rw_toupper (*s);
>> +                    *s = toupper (*s);
>>              }
>>  
>>              // if first character of new line is not whitespace, then we
>> have a new
>>              // canonical name token
>> -            if (!_rw_isspace (*key)) {
>> +            if (!isspace (*key)) {
>>  
>>                  canonical_name = key;
>>  
>>                  // increment key past cannonical name
>>                  for (/**/; *key; ++key)
>> -                    if (_rw_isspace (*key))
>> +                    if (isspace (*key))
>>                          break;
>>              }
>>  
>>              // kill whitespace
>> -            while (_rw_isspace (*key))
>> +            while (isspace (*key))
>>                  *key++ = '\0';
>>  
>>              // key points to first non-whitespace after canonical name
>> @@ -1582,11 +1529,11 @@
>>                      *key++ = '\0';
>>  
>>                  // kill any whitespace before comma
>> -                for (char* bey = key - 1; _rw_isspace (*bey); --bey)
>> +                for (char* bey = key - 1; isspace (*bey); --bey)
>>                      *bey = '\0';
>>  
>>                  // kill whitespace after comma
>> -                while (_rw_isspace (*key))
>> +                while (isspace (*key))
>>                      *key++ = '\0';
>>  
>>                  // ensure we have enough entries
>> 
>> 
>> 
> 
> 
> 

-- 
View this message in context: http://www.nabble.com/Re%3A-svn-commit%3A-r642397---in--stdcxx-trunk-tests-src%3A-braceexp.cpp-locale.cpp-tp16375460p16421931.html
Sent from the stdcxx-dev mailing list archive at Nabble.com.


Re: svn commit: r642397 - in /stdcxx/trunk/tests/src: braceexp.cpp locale.cpp

Posted by Martin Sebor <se...@roguewave.com>.
vitek@apache.org wrote:
> Author: vitek
> Date: Fri Mar 28 14:28:41 2008
> New Revision: 642397
> 
> URL: http://svn.apache.org/viewvc?rev=642397&view=rev
> Log:
> 
> 2008-03-28  Travis Vitek  <vi...@roguewave.com>
> 
> 	STDCXX-714
> 	* tests/src/braceexp.cpp: Remove _rw_isspace(), _rw_isupper() and
> 	_rw_islower(). Use appropriate C library calls instead.
> 
> 	STDCXX-716
> 	* tests/src/locale.cpp: Remove _rw_isspace(), _rw_toupper() and
> 	_rw_tolower(). Use appropriate C library calls instead.


Travis, the isxxx() functions take an int argument and that their
domain is the values between 0 and UCHAR_MAX plus EOF (and their
behavior is undefined otherwise -- IIRC, the Microsoft functions
abort in the undefined case). When passed a plain char, the
argument is subject to sign extension where the type signed and
values between CHAR_MIN and -1 are invalid. To make sure the
functions always behave correctly it's best to cast the argument
to unsigned char.

Martin

> 	
> 
> Modified:
>     stdcxx/trunk/tests/src/braceexp.cpp
>     stdcxx/trunk/tests/src/locale.cpp
> 
> Modified: stdcxx/trunk/tests/src/braceexp.cpp
> URL: http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/braceexp.cpp?rev=642397&r1=642396&r2=642397&view=diff
> ==============================================================================
> --- stdcxx/trunk/tests/src/braceexp.cpp (original)
> +++ stdcxx/trunk/tests/src/braceexp.cpp Fri Mar 28 14:28:41 2008
> @@ -3,55 +3,14 @@
>  
>  #include <stdlib.h> // for malloc(), free()
>  #include <string.h> // for memcpy()
> +#include <ctype.h>  // for isspace()
>  #include <assert.h> // for assert()
>  
>  #include <rw_braceexp.h>
>  
> -inline int _rw_is_lower (int ch)
> -{
> -    switch (ch) {
> -    case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> -    case 'g': case 'h': case 'i': case 'j': case 'k': case 'l':
> -    case 'm': case 'n': case 'o': case 'p': case 'q': case 'r':
> -    case 's': case 't': case 'u': case 'v': case 'w': case 'x':
> -    case 'y': case 'z':
> -        return 1;
> -    }
> -
> -    return 0;
> -}
> -
> -inline int _rw_is_upper (int ch)
> -{
> -    switch (ch) {
> -    case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> -    case 'G': case 'H': case 'I': case 'J': case 'K': case 'L':
> -    case 'M': case 'N': case 'O': case 'P': case 'Q': case 'R':
> -    case 'S': case 'T': case 'U': case 'V': case 'W': case 'X':
> -    case 'Y': case 'Z':
> -        return 1;
> -    }
> -
> -    return 0;
> -}
> -
> -inline int _rw_is_space (int ch)
> -{
> -    switch (ch)
> -    {
> -    case '\n':
> -    case '\r':
> -    case '\t':
> -    case ' ':
> -        return 1;
> -    }
> -
> -    return 0;
> -}
> -
>  inline int _rw_is_not_space (int ch)
>  {
> -    return !_rw_is_space (ch);
> +    return !isspace (ch);
>  }
>  
>  // search `beg' to `end' for a character that `fn'
> @@ -696,8 +655,8 @@
>      char cend = beg [4];
>  
>      // only works if sequence characters are both lowercase or uppercase.
> -    const int both_are_lower = _rw_is_lower (cbeg) && _rw_is_lower (cend);
> -    const int both_are_upper = _rw_is_upper (cbeg) && _rw_is_upper (cend);
> +    const int both_are_lower = islower (cbeg) && islower (cend);
> +    const int both_are_upper = isupper (cbeg) && isupper (cend);
>  
>      if (! (both_are_lower || both_are_upper))
>          return 0;
> @@ -1050,7 +1009,7 @@
>  
>      while (tok_beg)
>      {
> -        const char* tok_end = _rw_find_match (tok_beg, end, _rw_is_space);
> +        const char* tok_end = _rw_find_match (tok_beg, end, isspace);
>          if (!tok_end)
>              tok_end = end;
>  
> 
> Modified: stdcxx/trunk/tests/src/locale.cpp
> URL: http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/locale.cpp?rev=642397&r1=642396&r2=642397&view=diff
> ==============================================================================
> --- stdcxx/trunk/tests/src/locale.cpp (original)
> +++ stdcxx/trunk/tests/src/locale.cpp Fri Mar 28 14:28:41 2008
> @@ -951,59 +951,6 @@
>      struct _rw_locale_entry* next;
>  };
>  
> -static int
> -_rw_toupper (int chr)
> -{
> -    //if (chr < 'a' || 'z' < chr)
> -    //    return chr;
> -    //return chr - 'a' + 'A';
> -    switch (chr)
> -    {
> -    case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> -    case 'g': case 'h': case 'i': case 'j': case 'k': case 'l':
> -    case 'm': case 'n': case 'o': case 'p': case 'q': case 'r':
> -    case 's': case 't': case 'u': case 'v': case 'w': case 'x':
> -    case 'y': case 'z':
> -        return chr - 'a' + 'A';
> -    }
> -
> -    return chr;
> -}
> -
> -static int
> -_rw_tolower (int chr)
> -{
> -    //if (chr < 'A' || 'Z' < chr)
> -    //    return chr;
> -    //return chr - 'A' + 'a';
> -    switch (chr)
> -    {
> -    case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> -    case 'G': case 'H': case 'I': case 'J': case 'K': case 'L':
> -    case 'M': case 'N': case 'O': case 'P': case 'Q': case 'R':
> -    case 'S': case 'T': case 'U': case 'V': case 'W': case 'X':
> -    case 'Y': case 'Z':
> -        return chr - 'A' + 'a';
> -    }
> -
> -    return chr;
> -}
> -
> -static int
> -_rw_isspace (int chr)
> -{
> -    switch (chr)
> -    {
> -        case '\r':
> -        case '\n':
> -        case '\t':
> -        case ' ':
> -            return 1;
> -    }
> -
> -    return 0;
> -}
> -
>  struct _rw_locale_array {
>      _rw_locale_entry* entries;
>      _RWSTD_SIZE_T count;
> @@ -1205,7 +1152,7 @@
>              for (const char* charset = nl_langinfo (CODESET);
>                   *charset;
>                   ++charset) {
> -                codeset [i++] = _rw_toupper (*charset);
> +                codeset [i++] = toupper (*charset);
>              }
>  
>              codeset [i] = '\0';
> @@ -1225,7 +1172,7 @@
>                  *encoding++ = '\0';
>  
>                  for (int n = 0; encoding [n]; ++n)
> -                    encoding [n] = _rw_toupper (encoding [n]);
> +                    encoding [n] = toupper (encoding [n]);
>              }
>  
>              char* country = strrchr (locale, '_');
> @@ -1233,13 +1180,13 @@
>                  *country++ = '\0';
>  
>                  for (int n = 0; country [n]; ++n)
> -                    country [n] = _rw_toupper (country [n]);
> +                    country [n] = toupper (country [n]);
>              }
>              
>              char* language = locale;
>  
>              for (int n = 0; language [n]; ++n)
> -                language [n] = _rw_tolower (language [n]);
> +                language [n] = tolower (language [n]);
>  
>              // use mapping databases to find the canonical
>              // names for each part of the locale name
> @@ -1296,7 +1243,7 @@
>  
>              // the canonical name for lookup
>              sprintf (entry->canonical_name, "%s-%s-%d-%s",
> -                     planguage, pcountry, MB_CUR_MAX, pencoding);
> +                     planguage, pcountry, int (MB_CUR_MAX), pencoding);
>  
>              size += 1;
>          }
> @@ -1527,7 +1474,7 @@
>  
>              char* key = table_data + offset;
>  
> -            const int len = strcspn (key, "\r\n");
> +            const size_t len = strcspn (key, "\r\n");
>              key [len] = '\0';
>  
>              // skip the newline if it is there
> @@ -1541,27 +1488,27 @@
>              // make upper or lower case as requested
>              if (upper_or_lower < 0) {
>                  for (char* s = key; *s; ++s)
> -                    *s = _rw_tolower (*s);
> +                    *s = tolower (*s);
>              }
>              else if (0 < upper_or_lower) {
>                  for (char* s = key; *s; ++s)
> -                    *s = _rw_toupper (*s);
> +                    *s = toupper (*s);
>              }
>  
>              // if first character of new line is not whitespace, then we have a new
>              // canonical name token
> -            if (!_rw_isspace (*key)) {
> +            if (!isspace (*key)) {
>  
>                  canonical_name = key;
>  
>                  // increment key past cannonical name
>                  for (/**/; *key; ++key)
> -                    if (_rw_isspace (*key))
> +                    if (isspace (*key))
>                          break;
>              }
>  
>              // kill whitespace
> -            while (_rw_isspace (*key))
> +            while (isspace (*key))
>                  *key++ = '\0';
>  
>              // key points to first non-whitespace after canonical name
> @@ -1582,11 +1529,11 @@
>                      *key++ = '\0';
>  
>                  // kill any whitespace before comma
> -                for (char* bey = key - 1; _rw_isspace (*bey); --bey)
> +                for (char* bey = key - 1; isspace (*bey); --bey)
>                      *bey = '\0';
>  
>                  // kill whitespace after comma
> -                while (_rw_isspace (*key))
> +                while (isspace (*key))
>                      *key++ = '\0';
>  
>                  // ensure we have enough entries
> 
> 
> 


Re: svn commit: r642397 - in /stdcxx/trunk/tests/src: braceexp.cpp locale.cpp

Posted by Martin Sebor <se...@roguewave.com>.
FYI: I went ahead and fixed this to minimize the number of lost
build results over the rest of the weekend. Hope my changes are
okay with you: http://svn.apache.org/viewvc?rev=642646&view=rev

Martin

Martin Sebor wrote:
> vitek@apache.org wrote:
>> Author: vitek
>> Date: Fri Mar 28 14:28:41 2008
>> New Revision: 642397
>>
>> URL: http://svn.apache.org/viewvc?rev=642397&view=rev
>> Log:
>>
>> 2008-03-28  Travis Vitek  <vi...@roguewave.com>
>>
>>     STDCXX-714
>>     * tests/src/braceexp.cpp: Remove _rw_isspace(), _rw_isupper() and
>>     _rw_islower(). Use appropriate C library calls instead.
>>
>>     STDCXX-716
>>     * tests/src/locale.cpp: Remove _rw_isspace(), _rw_toupper() and
>>     _rw_tolower(). Use appropriate C library calls instead.
>>     
>>
>> Modified:
>>     stdcxx/trunk/tests/src/braceexp.cpp
>>     stdcxx/trunk/tests/src/locale.cpp
>>
>> Modified: stdcxx/trunk/tests/src/braceexp.cpp
>> URL: 
>> http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/braceexp.cpp?rev=642397&r1=642396&r2=642397&view=diff 
>>
>> ============================================================================== 
>>
>> --- stdcxx/trunk/tests/src/braceexp.cpp (original)
>> +++ stdcxx/trunk/tests/src/braceexp.cpp Fri Mar 28 14:28:41 2008
> [...]
>> @@ -1050,7 +1009,7 @@
>>  
>>      while (tok_beg)
>>      {
>> -        const char* tok_end = _rw_find_match (tok_beg, end, 
>> _rw_is_space);
>> +        const char* tok_end = _rw_find_match (tok_beg, end, isspace);
> 
> This is causing errors in EDG eccp builds (and likely many others
> with strictly conforming compilers) because of the incompatibility
> between functions with different language linkage. Also, there are
> (or may be) non-conforming platforms that shadow isspace and the
> other C functions with macros. I think FreeBSD is one them.
> 
> "/home/sebor/stdcxx/tests/src/braceexp.cpp", line 1012: error #167: 
> argument
>           of type "int (*)(int) C" is incompatible with parameter of type
>           "int (*)(int)"
>           const char* tok_end = _rw_find_match (tok_beg, end, isspace);
>                                                               ^
> Martin
> 
> 


Re: svn commit: r642397 - in /stdcxx/trunk/tests/src: braceexp.cpp locale.cpp

Posted by Martin Sebor <se...@roguewave.com>.
vitek@apache.org wrote:
> Author: vitek
> Date: Fri Mar 28 14:28:41 2008
> New Revision: 642397
> 
> URL: http://svn.apache.org/viewvc?rev=642397&view=rev
> Log:
> 
> 2008-03-28  Travis Vitek  <vi...@roguewave.com>
> 
> 	STDCXX-714
> 	* tests/src/braceexp.cpp: Remove _rw_isspace(), _rw_isupper() and
> 	_rw_islower(). Use appropriate C library calls instead.
> 
> 	STDCXX-716
> 	* tests/src/locale.cpp: Remove _rw_isspace(), _rw_toupper() and
> 	_rw_tolower(). Use appropriate C library calls instead.
> 	
> 
> Modified:
>     stdcxx/trunk/tests/src/braceexp.cpp
>     stdcxx/trunk/tests/src/locale.cpp
> 
> Modified: stdcxx/trunk/tests/src/braceexp.cpp
> URL: http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/braceexp.cpp?rev=642397&r1=642396&r2=642397&view=diff
> ==============================================================================
> --- stdcxx/trunk/tests/src/braceexp.cpp (original)
> +++ stdcxx/trunk/tests/src/braceexp.cpp Fri Mar 28 14:28:41 2008
[...]
> @@ -1050,7 +1009,7 @@
>  
>      while (tok_beg)
>      {
> -        const char* tok_end = _rw_find_match (tok_beg, end, _rw_is_space);
> +        const char* tok_end = _rw_find_match (tok_beg, end, isspace);

This is causing errors in EDG eccp builds (and likely many others
with strictly conforming compilers) because of the incompatibility
between functions with different language linkage. Also, there are
(or may be) non-conforming platforms that shadow isspace and the
other C functions with macros. I think FreeBSD is one them.

"/home/sebor/stdcxx/tests/src/braceexp.cpp", line 1012: error #167: argument
           of type "int (*)(int) C" is incompatible with parameter of type
           "int (*)(int)"
           const char* tok_end = _rw_find_match (tok_beg, end, isspace);
                                                               ^
Martin