[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] add visitor for parsing int[KMGT] input str
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] add visitor for parsing int[KMGT] input string |
Date: |
Fri, 14 Dec 2012 14:20:48 +0100 |
On Wed, 12 Dec 2012 16:16:42 -0200
Eduardo Habkost <address@hidden> wrote:
> On Mon, Dec 10, 2012 at 10:33:06PM +0100, Igor Mammedov wrote:
> > Caller of visit_type_suffixed_int() have to specify
> > value of 'K' suffix using suffix_factor argument.
> > Example of selecting suffix_factor value:
> > * Kbytes: 1024
> > * Khz: 1000
> >
> > Signed-off-by: Igor Mammedov <address@hidden>
>
> Reviewed-by: Eduardo Habkost <address@hidden>
>
>
> I wonder if we could later introduce a visit_type_frequency() function
> that simply calls visit_type_suffixed_int(). This would allow us to use
> a 'frequency' type on QAPI, like the existing 'size' type we already
> have.
>
> I suggest having explicitly distinct types on QAPI because the 'size'
> type probably won't abort (and maybe it _can't_ abort, to keep
> compatibility) in case it finds a "100MB" string. Likewise, the
It won't accept MB with current code, but we could probably pass
something like custom suffix table {KHz => 1000, MHz=>1000000, ...} instead
of unit for variables that accept frequency, and a corresponding table for
sizes and whatever else if needed. Than we could use only
visit_type_suffixed_int() and avoid creating an extra boiler code for every
kind of units that might be needed in future.
> 'frequency' type wouldn't abort in case it finds a "100MHz" string.
>
> With separate types, we could also make the 'frequency' type _not_
> accept "100B" as a valid string (strtosz_suffix_unit() accepts "B" as a
> valid suffix, today).
>
>
> > ---
> > v3:
> > - Fix errp check. Spotted-By: Andreas Färber <address@hidden>
> > - s/type_unit_suffixed_int/type_suffixed_int/
> > - use 'suffix_factor' instead of 'unit'
> > - document visit_type_suffixed_int()
> > - add comment on current impl. limitation
> > v2:
> > - convert type_freq to type_unit_suffixed_int.
> > - provide qapi_dealloc_type_unit_suffixed_int() impl.
> > ---
> > qapi/qapi-dealloc-visitor.c | 8 ++++++++
> > qapi/qapi-visit-core.c | 35 +++++++++++++++++++++++++++++++++++
> > qapi/qapi-visit-core.h | 4 ++++
> > qapi/string-input-visitor.c | 25 +++++++++++++++++++++++++
> > 4 files changed, 72 insertions(+)
> >
> [...]
>
> --
> Eduardo
>
--
Regards,
Igor