Re: changing ZLE_CHAR_T?

[Wayne Davison <wayned@xxxxxxxxxxxxxxxxxxxxx>]

On Sat, Oct 29, 2005 at 10:59:00AM +0100, Peter Stephenson wrote:
> you will need to be careful since those "int" definitions for the
> variables now marked as ZLE_CHAR_T have been around a very long time
> and it's not necessarily clear if they will hold an EOF or not, though
> if they do they should certainly now be ZLE_INT_T.

I looked at all the ZLE_CHAR_T variables while making my patch in order
to check for size differences (which is why a couple spots got changed
to use ZLE_INT_T).

One other potential problem area is less-than and greater-than
comparisons of character values now that the char may be signed.  I just
looked through the code to check for problems, and while I didn't see
any bugs, I did see a couple things that could be improved (such as
using idigit() instead of doing a range-comparison between '0' & '9').

Yet another potential problem area is calling functions such as
islower() with a signed character.  I looked through the code, and found
quite a few problems not related to my patch, including the calling of
some non-wide isTYPE() functions on wide characters in the zleline
array.  Another thing I noticed was that the wide version of ZC_iblank()
was calling iswspace(), which would return true for a newline, unlike
the normal iblank(), which does not (that is reserved for inblank()).

So, attached is a patch that fixes the problems I discovered:

I decided to add some new iTYPE() macros, namely iascii(), ilower(),
iprint(), and iupper().  I didn't modify inittyptab() to try to define
these values (for a number of reasons).  Instead, they ensure that
STOUC() gets called on the arg before we call the system's version of
these functions (aside: we could switch some of the existing macros over
to this method, such as idigit(), and free up some bits in the typtab[]
array).

I also added any missing ZC_iTYPE macros in zle.h, just so they'd all be
there.  The wide-char version of ZC_iblank now calls a new function,
wcsiblank(), that properly excludes '\n' from returning true.  However,
both ZC_iblank and ZC_inblank still make use of iswspace(), so they
return true for a carriage return ('\r') and a vertical tab ('\v') where
iblank and inblank do not.  I didn't try to change that one way or the
other.

Comments welcomed.

..wayne..

--- Src/utils.c	28 Oct 2005 17:34:33 -0000	1.101
+++ Src/utils.c	31 Oct 2005 21:22:57 -0000
@@ -2619,6 +2619,16 @@ inittyptab(void)
 
 
 #ifdef MULTIBYTE_SUPPORT
+/* A wide-character version of the iblank() macro. */
+/**/
+mod_export int
+wcsiblank(wint_t wc)
+{
+    if (iswspace(wc) && wc != L'\n')
+	return 1;
+    return 0;
+}
+
 /*
  * iword() macro extended to support wide characters.
  */
@@ -2641,7 +2651,7 @@ wcsiword(wchar_t c)
     if (len == 0) {
 	/* NULL is special */
 	return iword(0);
-    } else if (len == 1 && isascii(*outstr)) {
+    } else if (len == 1 && iascii(*outstr)) {
 	return iword(*outstr);
     } else {
 	return iswalnum(c) || wcschr(wordchars_wide, c);
@@ -2673,7 +2683,7 @@ wcsiident(wchar_t c)
     if (len == 0) {
 	/* NULL is special */
 	return 0;
-    } else if (len == 1 && isascii(*outstr)) {
+    } else if (len == 1 && iascii(*outstr)) {
 	return iident(*outstr);
     } else {
 	/* not currently allowed, see above */
--- Src/ztype.h	13 Oct 2005 16:30:14 -0000	1.2
+++ Src/ztype.h	31 Oct 2005 21:22:57 -0000
@@ -58,3 +58,8 @@
 #define imeta(X) _icom(X,IMETA)
 #define iwsep(X) _icom(X,IWSEP)
 #define inull(X) _icom(X,INULL)
+
+#define iascii(X) isascii(STOUC(X))
+#define ilower(X) islower(STOUC(X))
+#define iprint(X) isprint(STOUC(X))
+#define iupper(X) isupper(STOUC(X))
--- Src/Modules/zftp.c	6 Feb 2005 20:36:45 -0000	1.37
+++ Src/Modules/zftp.c	31 Oct 2005 21:22:58 -0000
@@ -713,8 +713,7 @@ zfgetmsg(void)
 
     zfgetline(line, 256, tmout);
     ptr = line;
-    if (zfdrrrring || !isdigit(STOUC(*ptr)) || !isdigit(STOUC(ptr[1])) || 
-	!isdigit(STOUC(ptr[2]))) {
+    if (zfdrrrring || !idigit(*ptr) || !idigit(ptr[1]) || !idigit(ptr[2])) {
 	/* timeout, or not talking FTP.  not really interested. */
 	zcfinish = 2;
 	if (!zfclosing)
@@ -914,7 +913,7 @@ zfopendata(char *name, union tcp_sockadd
 		goto bad_epsv;
 	    while(ptr != end && *ptr == '0')
 		ptr++;
-	    if(ptr == end || (end-ptr) > 5 || !isdigit(STOUC(*ptr)))
+	    if(ptr == end || (end-ptr) > 5 || !idigit(*ptr))
 		goto bad_epsv;
 	    memcpy(portbuf, ptr, (end-ptr));
 	    portbuf[end-ptr] = 0;
@@ -937,7 +936,7 @@ zfopendata(char *name, union tcp_sockadd
 	     * lastmsg already has the reply code expunged.
 	     */
 	    for (ptr = lastmsg; *ptr; ptr++)
-		if (isdigit(STOUC(*ptr)))
+		if (idigit(*ptr))
 		    break;
 	    if (sscanf(ptr, "%d,%d,%d,%d,%d,%d",
 		       nums, nums+1, nums+2, nums+3, nums+4, nums+5) != 6) {
@@ -1103,11 +1102,11 @@ zfgetdata(char *name, char *rest, char *
 	char *ptr = strstr(lastmsg, "bytes");
 	zfstatusp[zfsessno] |= ZFST_NOSZ|ZFST_TRSZ;
 	if (ptr) {
-	    while (ptr > lastmsg && !isdigit(STOUC(*ptr)))
+	    while (ptr > lastmsg && !idigit(*ptr))
 		ptr--;
-	    while (ptr > lastmsg && isdigit(STOUC(ptr[-1])))
+	    while (ptr > lastmsg && idigit(ptr[-1]))
 		ptr--;
-	    if (isdigit(STOUC(*ptr))) {
+	    if (idigit(*ptr)) {
 		zfstatusp[zfsessno] &= ~ZFST_NOSZ;
 		if (getsize) {
 		    off_t sz = zstrtol(ptr, NULL, 10);
--- Src/Modules/zselect.c	2 Jun 2004 22:15:01 -0000	1.6
+++ Src/Modules/zselect.c	31 Oct 2005 21:22:58 -0000
@@ -42,7 +42,7 @@ handle_digits(char *nam, char *argptr, f
     int fd;
     char *endptr;
 
-    if (!isdigit(STOUC(*argptr))) {
+    if (!idigit(*argptr)) {
 	zwarnnam(nam, "expecting file descriptor: %s", argptr, 0);
 	return 1;
     }
--- Src/Zle/zle.h	28 Oct 2005 17:34:33 -0000	1.21
+++ Src/Zle/zle.h	31 Oct 2005 21:22:58 -0000
@@ -70,13 +70,18 @@ typedef wint_t   ZLE_INT_T;
 #define ZMB_nicewidth(s)	mb_niceformat(s, NULL, NULL, 0)
 
 /* Functions that operate on ZLE_CHAR_T. */
-#define ZC_iblank iswspace
+#define ZC_ialpha iswalpha
+#define ZC_iblank wcsiblank
 #define ZC_icntrl iswcntrl
+#define ZC_idigit iswdigit
 #define ZC_iident wcsiident
+#define ZC_ilower iswlower
+#define ZC_inblank iswspace
+#define ZC_iupper iswupper
+#define ZC_iword wcsiword
 
 #define ZC_tolower towlower
 #define ZC_toupper towupper
-#define ZC_iword  wcsiword
 
 #define ZC_nicechar(c) wcs_nicechar(c, NULL, NULL)
 
@@ -129,13 +134,18 @@ static inline int ZS_strncmp(ZLE_STRING_
 #endif
 
 /* Functions that operate on ZLE_CHAR_T. */
+#define ZC_ialpha ialpha
 #define ZC_iblank iblank
 #define ZC_icntrl icntrl
+#define ZC_idigit idigit
 #define ZC_iident iident
+#define ZC_ilower ilower
+#define ZC_inblank inblank
+#define ZC_iupper iupper
+#define ZC_iword iword
 
 #define ZC_tolower tulower
 #define ZC_toupper tuupper
-#define ZC_iword   iword
 
 #define LASTFULLCHAR	lastchar
 
--- Src/Zle/zle_move.c	31 Oct 2005 21:19:30 -0000	1.7
+++ Src/Zle/zle_move.c	31 Oct 2005 21:22:58 -0000
@@ -459,7 +459,7 @@ int
 vifirstnonblank(UNUSED(char **args))
 {
     zlecs = findbol();
-    while (zlecs != zlell && iblank(zleline[zlecs]))
+    while (zlecs != zlell && ZC_iblank(zleline[zlecs]))
 	zlecs++;
     return 0;
 }
--- Src/Zle/zle_vi.c	28 Oct 2005 17:34:33 -0000	1.12
+++ Src/Zle/zle_vi.c	31 Oct 2005 21:22:59 -0000
@@ -487,7 +487,8 @@ vireplace(UNUSED(char **args))
 int
 vireplacechars(UNUSED(char **args))
 {
-    int ch, n = zmult;
+    ZLE_INT_T ch;
+    int n = zmult;
 
     startvichange(1);
     /* check argument range */
@@ -507,7 +508,7 @@ vireplacechars(UNUSED(char **args))
 	return 1;
     }
     /* do change */
-    if (ch == '\r' || ch == '\n') {
+    if (ch == ZWC('\r') || ch == ZWC('\n')) {
 	/* <return> handled specially */
 	zlecs += n - 1;
 	backkill(n - 1, 0);
@@ -570,9 +571,9 @@ vioperswapcase(UNUSED(char **args))
 	oldcs = zlecs;
 	/* swap the case of all letters within range */
 	while (zlecs < c2) {
-	    if (islower(zleline[zlecs]))
+	    if (ZC_ilower(zleline[zlecs]))
 		zleline[zlecs] = ZC_toupper(zleline[zlecs]);
-	    else if (isupper(zleline[zlecs]))
+	    else if (ZC_iupper(zleline[zlecs]))
 		zleline[zlecs] = ZC_tolower(zleline[zlecs]);
 	    zlecs++;
 	}
@@ -788,9 +789,9 @@ vijoin(UNUSED(char **args))
     if ((x = findeol()) == zlell)
 	return 1;
     zlecs = x + 1;
-    for (x = 1; zlecs != zlell && iblank(zleline[zlecs]); zlecs++, x++);
+    for (x = 1; zlecs != zlell && ZC_iblank(zleline[zlecs]); zlecs++, x++);
     backdel(x);
-    if (zlecs && iblank(zleline[zlecs-1]))
+    if (zlecs && ZC_iblank(zleline[zlecs-1]))
 	zlecs--;
     else {
 	spaceinline(1);
@@ -810,9 +811,9 @@ viswapcase(UNUSED(char **args))
 	return 1;
     eol = findeol();
     while (zlecs < eol && n--) {
-	if (islower(zleline[zlecs]))
+	if (ZC_ilower(zleline[zlecs]))
 	    zleline[zlecs] = ZC_toupper(zleline[zlecs]);
-	else if (isupper(zleline[zlecs]))
+	else if (ZC_iupper(zleline[zlecs]))
 	    zleline[zlecs] = ZC_tolower(zleline[zlecs]);
 	zlecs++;
     }
@@ -830,11 +831,7 @@ vicapslockpanic(UNUSED(char **args))
     statusline = ZWS("press a lowercase key to continue");
     statusll = ZS_strlen(statusline);
     zrefresh();
-#ifdef MULTIBYTE_SUPPORT
-    while (!iswlower(getfullchar(0)));
-#else
-    while (!islower(getfullchar(0)));
-#endif
+    while (!ZC_ilower(getfullchar(0)));
     statusline = NULL;
     return 0;
 }
--- Src/Zle/zle_word.c	9 Sep 2005 20:34:43 -0000	1.6
+++ Src/Zle/zle_word.c	31 Oct 2005 21:22:59 -0000
@@ -72,11 +72,11 @@ viforwardword(char **args)
 	    while (zlecs != zlell && iident(zleline[zlecs]))
 		zlecs++;
 	else
-	    while (zlecs != zlell && !iident(zleline[zlecs]) && !iblank(zleline[zlecs]))
+	    while (zlecs != zlell && !iident(zleline[zlecs]) && !ZC_iblank(zleline[zlecs]))
 		zlecs++;
 	if (wordflag && !n)
 	    return 0;
-	while (zlecs != zlell && (iblank(zleline[zlecs]) || zleline[zlecs] == '\n'))
+	while (zlecs != zlell && ZC_inblank(zleline[zlecs]))
 	    zlecs++;
     }
     return 0;
@@ -96,11 +96,11 @@ viforwardblankword(char **args)
 	return ret;
     }
     while (n--) {
-	while (zlecs != zlell && !iblank(zleline[zlecs]))
+	while (zlecs != zlell && !ZC_iblank(zleline[zlecs]))
 	    zlecs++;
 	if (wordflag && !n)
 	    return 0;
-	while (zlecs != zlell && iblank(zleline[zlecs]))
+	while (zlecs != zlell && ZC_iblank(zleline[zlecs]))
 	    zlecs++;
     }
     return 0;
@@ -139,9 +139,9 @@ viforwardblankwordend(UNUSED(char **args
     if (n < 0)
 	return 1;
     while (n--) {
-	while (zlecs != zlell && iblank(zleline[zlecs + 1]))
+	while (zlecs != zlell && ZC_iblank(zleline[zlecs + 1]))
 	    zlecs++;
-	while (zlecs != zlell && !iblank(zleline[zlecs + 1]))
+	while (zlecs != zlell && !ZC_iblank(zleline[zlecs + 1]))
 	    zlecs++;
     }
     if (zlecs != zlell && virangeflag)
@@ -163,14 +163,14 @@ viforwardwordend(char **args)
 	return ret;
     }
     while (n--) {
-	if (iblank(zleline[zlecs + 1]))
-	    while (zlecs != zlell && iblank(zleline[zlecs + 1]))
+	if (ZC_iblank(zleline[zlecs + 1]))
+	    while (zlecs != zlell && ZC_iblank(zleline[zlecs + 1]))
 		zlecs++;
 	if (iident(zleline[zlecs + 1]))
 	    while (zlecs != zlell && iident(zleline[zlecs + 1]))
 		zlecs++;
 	else
-	    while (zlecs != zlell && !iident(zleline[zlecs + 1]) && !iblank(zleline[zlecs + 1]))
+	    while (zlecs != zlell && !iident(zleline[zlecs + 1]) && !ZC_iblank(zleline[zlecs + 1]))
 		zlecs++;
     }
     if (zlecs != zlell && virangeflag)
@@ -214,13 +214,13 @@ vibackwardword(char **args)
 	return ret;
     }
     while (n--) {
-	while (zlecs && iblank(zleline[zlecs - 1]))
+	while (zlecs && ZC_iblank(zleline[zlecs - 1]))
 	    zlecs--;
 	if (iident(zleline[zlecs - 1]))
 	    while (zlecs && iident(zleline[zlecs - 1]))
 		zlecs--;
 	else
-	    while (zlecs && !iident(zleline[zlecs - 1]) && !iblank(zleline[zlecs - 1]))
+	    while (zlecs && !iident(zleline[zlecs - 1]) && !ZC_iblank(zleline[zlecs - 1]))
 		zlecs--;
     }
     return 0;
@@ -240,9 +240,9 @@ vibackwardblankword(char **args)
 	return ret;
     }
     while (n--) {
-	while (zlecs && iblank(zleline[zlecs - 1]))
+	while (zlecs && ZC_iblank(zleline[zlecs - 1]))
 	    zlecs--;
-	while (zlecs && !iblank(zleline[zlecs - 1]))
+	while (zlecs && !ZC_iblank(zleline[zlecs - 1]))
 	    zlecs--;
     }
     return 0;
@@ -304,13 +304,13 @@ vibackwardkillword(UNUSED(char **args))
 	return 1;
 /* this taken from "vibackwardword" */
     while (n--) {
-	while ((x > lim) && iblank(zleline[x - 1]))
+	while ((x > lim) && ZC_iblank(zleline[x - 1]))
 	    x--;
 	if (iident(zleline[x - 1]))
 	    while ((x > lim) && iident(zleline[x - 1]))
 		x--;
 	else
-	    while ((x > lim) && !iident(zleline[x - 1]) && !iblank(zleline[x - 1]))
+	    while ((x > lim) && !iident(zleline[x - 1]) && !ZC_iblank(zleline[x - 1]))
 		x--;
     }
     backkill(zlecs - x, 1);
@@ -398,7 +398,7 @@ capitalizeword(UNUSED(char **args))
 	first = 1;
 	while (zlecs != zlell && !ZC_iword(zleline[zlecs]))
 	    zlecs++;
-	while (zlecs != zlell && ZC_iword(zleline[zlecs]) && !isalpha(zleline[zlecs]))
+	while (zlecs != zlell && ZC_iword(zleline[zlecs]) && !ZC_ialpha(zleline[zlecs]))
 	    zlecs++;
 	while (zlecs != zlell && ZC_iword(zleline[zlecs])) {
 	    zleline[zlecs] = (first) ? ZC_toupper(zleline[zlecs]) :