Re: [PATCH 1/3] src: fix build warnings

["Jun. T" <takimoto-j@kba.biglobe.ne.jp>]

I've pushed the patch for pattern.c.

> 2021/03/28 4:28, Lawrence Velázquez <vq@larryv.me> wrote:
> 
> On Mon, Jan 4, 2021, at 12:14 AM, Daniel Shahaf wrote:
>> Felipe Contreras wrote on Sat, 02 Jan 2021 12:30 -0600:
>>> The best fix is to allocate one byte, especially if it's going to be
>>> freed later on.
>> 
>> Yes, please revise the patch to allocate byte(s) from the heap as
>> needed.  (See comments at the top of mem.c, and, for instance,
>> dupstring() v. ztrdup().)
> 
> Any plans to send a revised patch?


utils.c is more complicated.

getkeystring() returns a pointer to a string allocated from heap if
GETKEY_SINGLE_CHAR is not set.

But if it is set, it tries to find a next character (possibly multibyte)
in the given string, and returns the pointer to the byte following
the found character in the string.
There is no problem (aside from the compiler warning) if it finds the
next character.

But getkeystring() does return the pointer to tmp[] if it can't find
a next character.

Currently, GETKEY_SINGLE_CHAR is used only in one place in the zsh
source code; line 850 in math.c, in function zzlex():

                    ptr = getkeystring(ptr, NULL, GETKEYS_MATH, &v);

    GETKEYS_MATH is defined as
	(GETKEY_OCTAL_ESC|GETKEY_EMACS|GETKEY_CTRL|GETKEY_SINGLE_CHAR)

The code is used for finding a character after ## in arithmetic evaluation.
For example,

% echo $((##\M))
0

in this case 'ptr' does points to tmp[], and continues to be used for
further parsing. Although it seems we do not get coredump, I think this
needs be fixed.

In the patch below, I modified getkeystring() to return NULL if it can't
find the next character, and math.h to check the return value.



diff --git a/Src/math.c b/Src/math.c
index b57ba42d4..1d0d86639 100644
--- a/Src/math.c
+++ b/Src/math.c
@@ -840,13 +840,18 @@ zzlex(void)
 	    if (*ptr == '#') {
 		if (*++ptr == '\\' || *ptr == '#') {
 		    int v;
+		    char *optr = ptr;
 
 		    ptr++;
 		    if (!*ptr) {
 			zerr("bad math expression: character missing after ##");
 			return EOI;
 		    }
-		    ptr = getkeystring(ptr, NULL, GETKEYS_MATH, &v);
+		    if(!(ptr = getkeystring(ptr, NULL, GETKEYS_MATH, &v))) {
+			zerr("bad math expression: bad character after ##");
+			ptr = optr;
+			return EOI;
+		    }
 		    yyval.u.l = v;
 		    return NUM;
 		}
diff --git a/Src/utils.c b/Src/utils.c
index 1ac064a4e..5a9222919 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -6697,13 +6697,21 @@ ucs4toutf8(char *dest, unsigned int wval)
  *
  * The return value is unmetafied unless GETKEY_DOLLAR_QUOTE is
  * in use.
+ *
+ * If GETKEY_SINGLE_CHAR is set in how, a next character in the given
+ * string is parsed, and the character code for it is returned in misc.
+ * The return value of the function is a pointer to the byte in the
+ * given string from where the next parsing should start. If the next
+ * character can't be found then NULL is returned.
+ * CAUTION: Currently, GETKEY_SINGLE_CHAR can be used only via
+ *          GETKEYS_MATH. Other use of it may cause trouble.
  */
 
 /**/
 mod_export char *
 getkeystring(char *s, int *len, int how, int *misc)
 {
-    char *buf, tmp[1];
+    char *buf = NULL, tmp[1];
     char *t, *tdest = NULL, *u = NULL, *sstart = s, *tbuf = NULL;
     char svchar = '\0';
     int meta = 0, control = 0, ignoring = 0;
@@ -6729,9 +6737,11 @@ getkeystring(char *s, int *len, int how, int *misc)
     DPUTS((how & (GETKEY_DOLLAR_QUOTE|GETKEY_SINGLE_CHAR)) ==
 	  (GETKEY_DOLLAR_QUOTE|GETKEY_SINGLE_CHAR),
 	  "BUG: incompatible options in getkeystring");
+    DPUTS((how & GETKEY_SINGLE_CHAR) && (how != GETKEYS_MATH),
+	  "BUG: unsupported options in getkeystring");
 
     if (how & GETKEY_SINGLE_CHAR)
-	t = buf = tmp;
+	t = tmp;
     else {
 	/* Length including terminating NULL */
 	int maxlen = 1;
@@ -7165,13 +7175,20 @@ getkeystring(char *s, int *len, int how, int *misc)
      */
     DPUTS((how & (GETKEY_DOLLAR_QUOTE|GETKEY_UPDATE_OFFSET)) ==
 	  GETKEY_DOLLAR_QUOTE, "BUG: unterminated $' substitution");
-    *t = '\0';
-    if (how & GETKEY_DOLLAR_QUOTE)
-	*tdest = '\0';
-    if (how & GETKEY_SINGLE_CHAR)
+
+    if (how & GETKEY_SINGLE_CHAR) {
+	/* couldn't find a character */
 	*misc = 0;
-    else
-	*len = ((how & GETKEY_DOLLAR_QUOTE) ? tdest : t) - buf;
+	return NULL;
+    }
+    if (how & GETKEY_DOLLAR_QUOTE) {
+	*tdest = '\0';
+	*len = tdest - buf;
+    }
+    else {
+	*t = '\0';
+	*len = t - buf;
+    }
     return buf;
 }