From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34647) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tj8S4-0006uu-9m for qemu-devel@nongnu.org; Thu, 13 Dec 2012 08:03:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tj8Rz-0005vv-3n for qemu-devel@nongnu.org; Thu, 13 Dec 2012 08:03:48 -0500 Received: from mx1.redhat.com ([209.132.183.28]:21913) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tj8Ry-0005vm-Qz for qemu-devel@nongnu.org; Thu, 13 Dec 2012 08:03:43 -0500 From: Markus Armbruster References: <1354852190-15095-1-git-send-email-lig.fnst@cn.fujitsu.com> <20121212111203.2cef65d9@nial.usersys.redhat.com> <87bodygyzd.fsf@blackfin.pond.sub.org> <20121213101106.658c554e@thinkpad.mammed.net> Date: Thu, 13 Dec 2012 13:09:56 +0100 In-Reply-To: <20121213101106.658c554e@thinkpad.mammed.net> (Igor Mammedov's message of "Thu, 13 Dec 2012 10:11:06 +0100") Message-ID: <87d2yedugb.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [ [PATCH 1/2] cutils:change strtosz_suffix_unit function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: kwolf@redhat.com, ehabkost@redhat.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, stefanha@redhat.com, liguang Igor Mammedov writes: > On Thu, 13 Dec 2012 09:03:50 +0100 > Markus Armbruster wrote: > >> Igor Mammedov writes: >> >> > On Fri, 7 Dec 2012 11:49:49 +0800 >> > liguang wrote: >> > >> >> if value to be translated is larger than INT64_MAX, >> >> this function will not be convenient for caller to >> >> be aware of it, so change a little for this. >> >> >> >> Signed-off-by: liguang >> >> --- >> >> cutils.c | 5 +++-- >> >> 1 files changed, 3 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/cutils.c b/cutils.c >> >> index 4f0692f..8905b5e 100644 >> >> --- a/cutils.c >> >> +++ b/cutils.c >> >> @@ -219,11 +219,11 @@ static int64_t suffix_mul(char suffix, int64_t unit) >> >> int64_t strtosz_suffix_unit(const char *nptr, char **end, >> >> const char default_suffix, int64_t unit) >> >> { >> >> - int64_t retval = -1; >> >> + int64_t retval = -1, mul; >> >> char *endptr; >> >> unsigned char c; >> >> int mul_required = 0; >> >> - double val, mul, integral, fraction; >> >> + double val, integral, fraction; >> >> >> >> errno = 0; >> >> val = strtod(nptr, &endptr); >> >> @@ -246,6 +246,7 @@ int64_t strtosz_suffix_unit(const char *nptr, char >> >> **end, goto fail; >> >> } >> >> if ((val * mul >= INT64_MAX) || val < 0) { >> >> + retval = 0; >> > Why not to add Error argument to return errors instead of using >> > return value? >> > That way function would be easier to generalize in future to handle whole >> > INT64 range. This function parses *sizes*. Necessarily non-negative. It returns them as int64_t. Fine for sizes up to 2^63-1 bytes. >> Generalize when you have a user, not earlier. > http://permalink.gmane.org/gmane.comp.emulators.qemu/183792 Why does that user require sizes in [2^63..2^64-1]? >> > And callers would check only returned error instead of return >> > value or if 'end' == 0. >> >> Checking the return value is sufficient now. What makes you think you >> have to check end, too? > I've meant *endptr == "\0" check in several callers: > opts_type_size(), img_create(), numa_add(). That check tests something else, namely "no junk follows the number". You cannot put that test into strtosz_suffix_unit(), because different callers accept numbers in different contexts.