From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=43729 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OHEGV-0006NE-L0 for qemu-devel@nongnu.org; Wed, 26 May 2010 06:55:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OHEGU-00049X-98 for qemu-devel@nongnu.org; Wed, 26 May 2010 06:55:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39489) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OHEGT-00049I-Sg for qemu-devel@nongnu.org; Wed, 26 May 2010 06:55:10 -0400 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o4QAt8o9027399 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 26 May 2010 06:55:09 -0400 Message-ID: <4BFCFDF9.3090509@redhat.com> Date: Wed, 26 May 2010 12:54:49 +0200 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] qdev-properties: Fix (u)intXX parsers References: <1274869693-22884-1-git-send-email-kwolf@redhat.com> <20100526104558.GL18547@redhat.com> In-Reply-To: <20100526104558.GL18547@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: armbru@redhat.com, qemu-devel@nongnu.org, kraxel@redhat.com Am 26.05.2010 12:45, schrieb Daniel P. Berrange: > On Wed, May 26, 2010 at 12:28:13PM +0200, Kevin Wolf wrote: >> scanf calls must not use PRI constants, they have probably the wrong size and >> corrupt memory. We could replace them by SCN ones, but strtol is simpler than >> scanf here anyway. While at it, also fix the parsers to reject garbage after >> the number ("4096xyz" was accepted before). >> >> Signed-off-by: Kevin Wolf >> --- >> hw/qdev-properties.c | 50 +++++++++++++++++++++++++++++++++++--------------- >> 1 files changed, 35 insertions(+), 15 deletions(-) >> >> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c >> index 9ffdba7..9a61ca2 100644 >> --- a/hw/qdev-properties.c >> +++ b/hw/qdev-properties.c >> @@ -68,12 +68,14 @@ PropertyInfo qdev_prop_bit = { >> static int parse_uint8(DeviceState *dev, Property *prop, const char *str) >> { >> uint8_t *ptr = qdev_get_prop_ptr(dev, prop); >> - const char *fmt; >> + char *end; >> >> /* accept both hex and decimal */ >> - fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx8 : "%" PRIu8; >> - if (sscanf(str, fmt, ptr) != 1) >> + *ptr = strtoul(str, &end, 0); >> + if (end != str + strlen(str)) { >> return -EINVAL; >> + } > > I think you can avoid the O(n) operation here & in the other cases with > a test like this: > > if ((end == str) || (*end != '\0')) > return -EINVAL It probably doesn't really make a difference here, but you're right. I'll send another version with this change. Kevin