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
TravisMartin Sebor wrote:[EMAIL PROTECTED] 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 <[EMAIL PROTECTED]> 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. MartinModified: 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 oruppercase. - 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 lookupsprintf (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 notwhitespace, then wehave a new // canonical name token - if (!_rw_isspace (*key)) { + if (!isspace (*key)) {canonical_name = key; // increment key past cannonical namefor (/**/; *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 aftercanonical 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.
