From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=41574 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Ow8kY-0000EK-NC
	for qemu-devel@nongnu.org; Thu, 16 Sep 2010 03:19:19 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <pbonzini@redhat.com>) id 1Ow8kX-0003au-NV
	for qemu-devel@nongnu.org; Thu, 16 Sep 2010 03:19:18 -0400
Received: from mx1.redhat.com ([209.132.183.28]:36164)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <pbonzini@redhat.com>) id 1Ow8kX-0003ac-HF
	for qemu-devel@nongnu.org; Thu, 16 Sep 2010 03:19:17 -0400
Received: from int-mx03.intmail.prod.int.phx2.redhat.com
	(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16])
	by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o8G7JFvo029745
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <qemu-devel@nongnu.org>; Thu, 16 Sep 2010 03:19:16 -0400
Message-ID: <4C91C4F1.1010501@redhat.com>
Date: Thu, 16 Sep 2010 09:19:13 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1284553440-17985-1-git-send-email-Jes.Sorensen@redhat.com>
	<1284553440-17985-3-git-send-email-Jes.Sorensen@redhat.com>
	<4C90EA14.7040801@redhat.com> <4C911EFE.2020504@redhat.com>
In-Reply-To: <4C911EFE.2020504@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH 2/5] Support human unit formats in
	strtobytes, eg. 1.0G
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jes Sorensen <Jes.Sorensen@redhat.com>
Cc: quintela@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com

On 09/15/2010 09:31 PM, Jes Sorensen wrote:
> On 09/15/10 17:45, Paolo Bonzini wrote:
>> On 09/15/2010 02:23 PM, Jes.Sorensen@redhat.com wrote:
>>>        switch (*endptr++) {
>>>        case 'K':
>>>        case 'k':
>>>            value<<= 10;
>>>            break;
>>>        case 0:
>>> +        if (divider) {
>>> +            value = 0;
>>> +            break;
>>> +        }
>>>        case 'M':
>>>        case 'm':
>>>            value<<= 20;
>>> @@ -284,9 +306,12 @@ uint64_t strtobytes(const char *nptr, char **end)
>>>        default:
>>>            value = 0;
>>>        }
>>> +    if (divider)
>>> +        value /= divider;
>>>
>>
>> This risks overflow if you do 1.00000000000000G or something similarly
>> braindead.  Do we loathe floating point so much that you cannot use
>> strtod, like
>
> Floating point is just plain wrong. If someone wants to do something
> like in your example they really ask for an error.

An error, not an overflow.

Adding overflow checking on top of your patch is also fine.  Another 
possibility is to look ahead for the multiplier so that you correctly 
base the divider and do everything in 64.64 fixed point.  But it seems 
overkill compared to floating-point, whose 53-bit mantissa precision 
will almost always lead to exact results (large numbers usually have a 
lot of zeros at the end, both in binary and in decimal).

Paolo