From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JzDID-0002VH-9p for qemu-devel@nongnu.org; Thu, 22 May 2008 12:05:25 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JzDIC-0002U2-KE for qemu-devel@nongnu.org; Thu, 22 May 2008 12:05:24 -0400 Received: from [199.232.76.173] (port=40683 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JzDIC-0002Tk-D9 for qemu-devel@nongnu.org; Thu, 22 May 2008 12:05:24 -0400 Received: from bsdimp.com ([199.45.160.85]:55892 helo=harmony.bsdimp.com) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1JzDIB-0001QM-Vi for qemu-devel@nongnu.org; Thu, 22 May 2008 12:05:24 -0400 Date: Thu, 22 May 2008 10:05:09 -0600 (MDT) Message-Id: <20080522.100509.-1540404811.imp@bsdimp.com> Subject: Re: [Qemu-devel] Re: [PATCH 1/2] Refactor und fix do_sendkey From: "M. Warner Losh" In-Reply-To: <48359204.80200@web.de> References: <483571E7.2@web.de> <483587DD.6050502@bellard.org> <48359204.80200@web.de> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, jan.kiszka@web.de In message: <48359204.80200@web.de> Jan Kiszka writes: : Fabrice Bellard wrote: : > Please avoid strncpy. Use pstrcpy intead. : : Ah, non-POSIX host platforms. Wouldn't it be easier to provide POSIX : wrappers for those few OSes? strncpy is plain, vanilla C. However, its use is dangerous and very often used wrong. Warner : However, fixed-up version follows: : : ----------- : : Looking at the sendkey implementation, planning to enhance it with a : hold time argument, I found some potential out-of-bound access and not : very readable code. Here is a fix for the former and a (subjective) : improvement of the latter. : : Signed-off-by: Jan Kiszka : --- : monitor.c | 52 +++++++++++++++++++++++++++++----------------------- : 1 file changed, 29 insertions(+), 23 deletions(-) : : Index: b/monitor.c : =================================================================== : --- a/monitor.c : +++ b/monitor.c : @@ -925,33 +925,39 @@ static int get_keycode(const char *key) : return -1; : } : : -static void do_send_key(const char *string) : +static void do_sendkey(const char *string) : { : - char keybuf[16], *q; : uint8_t keycodes[16]; : - const char *p; : - int nb_keycodes, keycode, i; : - : - nb_keycodes = 0; : - p = string; : - while (*p != '\0') { : - q = keybuf; : - while (*p != '\0' && *p != '-') { : - if ((q - keybuf) < sizeof(keybuf) - 1) { : - *q++ = *p; : + int nb_keycodes = 0; : + char keyname_buf[16]; : + char *separator; : + int keyname_len, keycode, i; : + : + while (1) { : + separator = strchr(string, '-'); : + keyname_len = separator ? separator-string : strlen(string); : + if (keyname_len > 0) { : + pstrcpy(keyname_buf, string, sizeof(keyname_buf) - 1); : + if (keyname_len > sizeof(keyname_buf) - 1) { : + keyname_buf[sizeof(keyname_buf) - 1] = 0; : + term_printf("invalid key: '%s...'\n", keyname_buf); : + return; : } : - p++; : - } : - *q = '\0'; : - keycode = get_keycode(keybuf); : - if (keycode < 0) { : - term_printf("unknown key: '%s'\n", keybuf); : - return; : + if (nb_keycodes == sizeof(keycodes)) { : + term_printf("too many keys\n"); : + return; : + } : + keyname_buf[keyname_len] = 0; : + keycode = get_keycode(keyname_buf); : + if (keycode < 0) { : + term_printf("unknown key: '%s'\n", keyname_buf); : + return; : + } : + keycodes[nb_keycodes++] = keycode; : } : - keycodes[nb_keycodes++] = keycode; : - if (*p == '\0') : + if (!separator) : break; : - p++; : + string = separator + 1; : } : /* key down events */ : for(i = 0; i < nb_keycodes; i++) { : @@ -1353,7 +1359,7 @@ static term_cmd_t term_cmds[] = { : { "i", "/ii.", do_ioport_read, : "/fmt addr", "I/O port read" }, : : - { "sendkey", "s", do_send_key, : + { "sendkey", "s", do_sendkey, : "keys", "send keys to the VM (e.g. 'sendkey ctrl-alt-f1')" }, : { "system_reset", "", do_system_reset, : "", "reset the system" }, : : :