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.
>>
>>
>
|