On Sat, 06 Jan 2018 13:25:37 -0700, "Todd C. Miller" wrote:

> kb_add() counts the number of arguments passed via stdargs.
> This would make sense if the buffer was allocated dynamically
> but it is not.  As such, there's no need to loop over the
> arguments twice.
>
> Also, the loop that fills in the line buffer (l) doesn't ensure
> that we stay within the bounds of the buffer.  The following diff
> makes sure we don't overrun the buffer and also eliminates a dead
> store in x_redraw().

Update diff with input from anton@ that doesn't include the x_redraw()
bit.

 - todd

Index: bin/ksh/emacs.c
===================================================================
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.80
diff -u -p -u -r1.80 emacs.c
--- bin/ksh/emacs.c     6 Jan 2018 16:28:58 -0000       1.80
+++ bin/ksh/emacs.c     6 Jan 2018 23:49:29 -0000
@@ -1328,20 +1328,21 @@ static struct kb_entry *
 kb_add(void *func, void *args, ...)
 {
        va_list                 ap;
-       unsigned int            i, count = 0;
-       char                    l[LINE + 1];
+       unsigned char           ch;
+       unsigned int            i;
+       char                    line[LINE + 1];
 
        va_start(ap, args);
-       while (va_arg(ap, unsigned int) != 0)
-               count++;
+       for (i = 0; i < sizeof(line) - 1; i++) {
+               ch = va_arg(ap, unsigned int);
+               if (ch == 0)
+                       break;
+               line[i] = ch;
+       }
        va_end(ap);
+       line[i] = '\0';
 
-       va_start(ap, args);
-       for (i = 0; i <= count /* <= is correct */; i++)
-               l[i] = (unsigned char)va_arg(ap, unsigned int);
-       va_end(ap);
-
-       return (kb_add_string(func, args, l));
+       return (kb_add_string(func, args, line));
 }
 
 static void

Reply via email to