From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NXwuW-00078Z-8U for qemu-devel@nongnu.org; Thu, 21 Jan 2010 08:17:20 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NXwuR-00077c-MD for qemu-devel@nongnu.org; Thu, 21 Jan 2010 08:17:19 -0500 Received: from [199.232.76.173] (port=43484 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NXwuR-00077Z-EQ for qemu-devel@nongnu.org; Thu, 21 Jan 2010 08:17:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:3493) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NXwuQ-00082I-Io for qemu-devel@nongnu.org; Thu, 21 Jan 2010 08:17:15 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o0LDHC6C024889 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 21 Jan 2010 08:17:13 -0500 Date: Thu, 21 Jan 2010 11:17:03 -0200 From: Luiz Capitulino Subject: Re: [Qemu-devel] [PATCH v2 3/8] monitor: New argument type 'b' Message-ID: <20100121111703.50562cb2@doriath> In-Reply-To: <1264003702-17329-4-git-send-email-armbru@redhat.com> References: <1264003702-17329-1-git-send-email-armbru@redhat.com> <1264003702-17329-4-git-send-email-armbru@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org On Wed, 20 Jan 2010 17:08:17 +0100 Markus Armbruster wrote: > This is a double value with optional suffixes G, g, M, m, K, k. We'll > need this to get migrate_set_speed() QMP-ready. Nice, not only good for QMP: we're moving this kind of handling from the handlers to common code, which is the right thing to do. The only possible issue is that, if we decide to move all this stuff to json, such types will make the change complex. But that's something for the future. Some comments follow. > Signed-off-by: Markus Armbruster > --- > monitor.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 58 insertions(+), 0 deletions(-) > > diff --git a/monitor.c b/monitor.c > index 775fe3f..ce97e7b 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -47,6 +47,7 @@ > #include "kvm.h" > #include "acl.h" > #include "qint.h" > +#include "qfloat.h" > #include "qlist.h" > #include "qdict.h" > #include "qbool.h" > @@ -70,6 +71,10 @@ > * 'l' target long (32 or 64 bit) > * 'M' just like 'l', except in user mode the value is > * multiplied by 2^20 (think Mebibyte) > + * 'b' double > + * user mode accepts an optional G, g, M, m, K, k suffix, > + * which multiplies the value by 2^30 for suffixes G and > + * g, 2^20 for M and m, 2^10 for K and k > * '/' optional gdb-like print format (like "/10x") > * > * '?' optional type (for all types, except '/') > @@ -3181,6 +3186,27 @@ static int get_expr(Monitor *mon, int64_t *pval, const char **pp) > return 0; > } > > +static int get_double(Monitor *mon, double *pval, const char **pp) > +{ > + const char *p = *pp; > + char *tailp; Better to init to NULL? > + double d; > + > + errno = 0; > + d = strtod(p, &tailp); > + if (tailp == p) { > + monitor_printf(mon, "Number expected\n"); > + return -1; > + } > + if (errno) { > + monitor_printf(mon, "Bad number (%s)\n", strerror(errno)); > + return -1; > + } Should we trust errno this way? The manpage only mentions ERANGE. > + *pval = d; > + *pp = tailp; > + return 0; > +} > + > static int get_str(char *buf, int buf_size, const char **pp) > { > const char *p; > @@ -3517,6 +3543,38 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, > qdict_put(qdict, key, qint_from_int(val)); > } > break; > + case 'b': > + { > + double val; > + > + while (qemu_isspace(*p)) > + p++; > + if (*typestr == '?') { > + typestr++; > + if (*p == '\0') { > + break; > + } > + } > + if (get_double(mon, &val, &p) < 0) { > + goto fail; > + } > + if (*p) { > + switch (*p) { > + case 'K': case 'k': > + val *= 1 << 10; p++; break; > + case 'M': case 'm': > + val *= 1 << 20; p++; break; > + case 'G': case 'g': > + val *= 1 << 30; p++; break; > + } > + } > + if (*p && !qemu_isspace(*p)) { > + monitor_printf(mon, "Unknown unit suffix\n"); > + goto fail; > + } A good way to test if 'p' handling is correct, is to write a test handler which has different types (say, 'foo:b,str:s,bla:i') and print the values to see if they match what we expect or have hardcoded to values in a specific test handler... > + qdict_put(qdict, key, qfloat_from_double(val)); > + } > + break; > case '-': > { > const char *tmp = p;