Mail Archives: djgpp-workers/2001/12/27/05:51:45
Hi!
Richard Dawe <rich AT phekda DOT freeserve DOT co DOT uk> wrote:
>> I have redone the patch (regex.nls+warning.diff) by casting all ctype.h
>> function's arguments to unsigned char. Also I have fix the warnings
>> caused by the missed brackets and unused values calculated. There's 4
>> more warnings in pedantic mode about possible use of uninitialized
>> variables, but they seems to be safe to ignore.
>>
>> The second fix (regex.nls2.diff) is intended for the proper working in
>> NLS capable environment.
>
>I don't know much about NLS, but I have a few comments about these
>patches:
>
>* Could you use the '-p' option, when you do a diff please? That includes
>the function name in the header of each diff, which is helpful when
>looking at the code (for me, at least).
Ok.
>* Why do you cast int characters to unsigned char, when passing them to
>the ctype functions? Is this to ensure that the inline implementation of
>the ctype functions doesn't overrun the ctype buffers by looking up an
>invalid character (see include/inlines/ctype.ha)? Does the code without
>this patch crash because a buffer overrun?
Yes, it seems like infinite loops caused by the buffer over/under-run.
>* What do you mean by "proper working in NLS capable environment"? It
>looks like the code still only supports the C locale. Presumably you mean
>it doesn't crash, when locales other than the C one are being used by
>programs built against libc?
If (or when ;-)) my patch to the locale support will be incorporated in the DJGPP the ctype macroses will be capable to handle isalpha, etc. in more proper way than just for those letters which are defined in C locale.
>> --- regex.orig/regcomp.c Sat Jun 9 23:50:56 2001
>> +++ regex/regcomp.c Tue Dec 18 18:51:12 2001
>> @@ -53,10 +53,10 @@
>> #define NEXTn(n) (p->next += (n))
>> #define GETNEXT() (*p->next++)
>> #define SETERROR(e) seterr(p, (e))
>> -#define REQUIRE(co, e) ((co) || SETERROR(e))
>> -#define MUSTSEE(c, e) (REQUIRE(MORE() && PEEK() == (c), e))
>> -#define MUSTEAT(c, e) (REQUIRE(MORE() && GETNEXT() == (c), e))
>> -#define MUSTNOTSEE(c, e) (REQUIRE(!MORE() || PEEK() != (c), e))
>> +#define REQUIRE(co, e) { if(!(co)) SETERROR(e); }
>> +#define MUSTSEE(c, e) REQUIRE(MORE() && PEEK() == (c), e)
>> +#define MUSTEAT(c, e) REQUIRE(MORE() && GETNEXT() == (c), e)
>> +#define MUSTNOTSEE(c, e) REQUIRE(!MORE() || PEEK() != (c), e)
>> #define EMIT(op, sopnd) doemit(p, (sop)(op), (size_t)(sopnd))
>> #define INSERT(op, pos) doinsert(p, (sop)(op), HERE()-(pos)+1, pos)
>> #define AHEAD(pos) dofwd(p, pos, HERE()-(pos))
>
>Why did you remove the brackets around REQUIRE(...) in MUSTSEE, MUSTEAT
>and MUSTNOTSEE? Since they're macros, isn't it safer to enclose them in
>brackets, to avoid subtle bugs?
Because I've changed the REQUIRE itself :-) So in order to compile code brackets should be removed. This is safe from the point of subtle bugs caused by the missed brackets.
--
alexander aganichev
url: http://aaganichev.narod.ru
__________________________________________________________________
Your favorite stores, helpful shopping tools and great gift ideas. Experience the convenience of buying online with Shop AT Netscape! http://shopnow.netscape.com/
Get your own FREE, personal Netscape Mail account today at http://webmail.netscape.com/
- Raw text -