stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: svn commit: r642397 - in /stdcxx/trunk/tests/src: braceexp.cpp locale.cpp
Date Tue, 01 Apr 2008 20:33:01 GMT
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  <vitek@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.
>>
>>
> 


Mime
View raw message