From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33031) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ti5zR-000053-Uy for qemu-devel@nongnu.org; Mon, 10 Dec 2012 11:14:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ti5zL-0006ll-Nv for qemu-devel@nongnu.org; Mon, 10 Dec 2012 11:13:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:16654) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ti5zL-0006lf-GD for qemu-devel@nongnu.org; Mon, 10 Dec 2012 11:13:51 -0500 Date: Mon, 10 Dec 2012 17:13:45 +0100 From: Igor Mammedov Message-ID: <20121210171345.42b05f80@nial.usersys.redhat.com> In-Reply-To: <20121207200906.GH4255@otherpad.lan.raisama.net> References: <1354828325-16568-1-git-send-email-imammedo@redhat.com> <1354828325-16568-3-git-send-email-imammedo@redhat.com> <50C23CB9.2020508@suse.de> <20121207200906.GH4255@otherpad.lan.raisama.net> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/2] target-i386: use visit_type_unit_suffixed_int() to parse tsc_freq property value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: mdroth@linux.vnet.ibm.com, Andreas =?ISO-8859-1?B?RuRyYmVy?= , qemu-devel@nongnu.org On Fri, 7 Dec 2012 18:09:06 -0200 Eduardo Habkost wrote: > On Fri, Dec 07, 2012 at 08:00:09PM +0100, Andreas F=E4rber wrote: > > Am 06.12.2012 22:12, schrieb Igor Mammedov: > > > Signed-off-by: Igor Mammedov > > > --- > > > v2: > > > - replace visit_type_freq() with visit_type_unit_suffixed_int() > > > in x86_cpuid_set_tsc_freq() > > > --- > > > target-i386/cpu.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > >=20 > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > > index c6c2ca0..b7f0aba 100644 > > > --- a/target-i386/cpu.c > > > +++ b/target-i386/cpu.c > > > @@ -1195,7 +1195,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, > > > Visitor *v, void *opaque, const int64_t max =3D INT64_MAX; > > > int64_t value; > > > =20 > > > - visit_type_int(v, &value, name, errp); > > > + visit_type_unit_suffixed_int(v, &value, name, 1000, errp); > > > if (error_is_set(errp)) { > > > return; > > > } > >=20 > > This trivial usage is fine obviously. But since this series set out to > > make things more generic I am missing at least one use case for 1024. > > Does nothing like that exist in qdev-properties.c or so already? >=20 > cutils.c has: >=20 > int64_t strtosz_suffix(const char *nptr, char **end, const char > default_suffix) { > return strtosz_suffix_unit(nptr, end, default_suffix, 1024); > } >=20 > $ git grep -w strtosz_suffix > [...] > qapi/opts-visitor.c: val =3D strtosz_suffix(opt->str ? opt->str : "", > &endptr, qemu-img.c: sval =3D strtosz_suffix(argv[optind++], &end, > STRTOSZ_DEFSUFFIX_B); qemu-img.c: sval =3D strtosz_suffix(opta= rg, > &end, STRTOSZ_DEFSUFFIX_B); >=20 > The opts-visitor.c match, in turn, is inside opts_type_size(), that's the > ->type_size method of OptsVisitor. There are many 'size' elements inside > qapi-schema.json. >=20 > I don't see any code using visit_type_size() directly, but I see two users > of type 'size' on qapi-schema.json: NetdevTapOptions and NetdevDumpOption= s. >=20 > I didn't know that we already had a visitor method using the suffixed-int > parsing code. Should we change the visit_type_size() code to be to use use > the new generic ->type_suffixed_int method and kill ->type_size? If there isn't strong opposition to do it in incremental way, I'd prefer for these patches go in first. And then later fix users of visit_type_size() to use type_suffixed_int() or maybe have a new type_suffixed_uint() so that size could be represented as uint64_t instead of int64_t as it's now. That would require to rewrite strtosz_* and its callers a bit.