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

 - 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 20:20:11 -0000
@@ -1038,7 +1038,6 @@ x_redraw(int limit)
                x_displen = xx_cols - 2;
        }
        xlp_valid = false;
-       cp = x_lastcp();
        x_zots(xbp);
        if (xbp != xbuf || xep > xlp)
                limit = xx_cols;
@@ -1328,18 +1327,18 @@ static struct kb_entry *
 kb_add(void *func, void *args, ...)
 {
        va_list                 ap;
-       unsigned int            i, count = 0;
+       unsigned int            ch, i;
        char                    l[LINE + 1];
 
        va_start(ap, args);
-       while (va_arg(ap, unsigned int) != 0)
-               count++;
-       va_end(ap);
-
-       va_start(ap, args);
-       for (i = 0; i <= count /* <= is correct */; i++)
-               l[i] = (unsigned char)va_arg(ap, unsigned int);
+       for (i = 0; i < sizeof(l) - 1; i++) {
+               ch = (unsigned char)va_arg(ap, unsigned int);
+               if (ch == 0)
+                       break;
+               l[i] = ch;
+       }
        va_end(ap);
+       l[i] = '\0';
 
        return (kb_add_string(func, args, l));
 }

Reply via email to