* [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer
@ 2012-12-10 21:33 Igor Mammedov
2012-12-10 21:33 ` [Qemu-devel] [PATCH 1/2] add visitor for parsing int[KMGT] input string Igor Mammedov
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Igor Mammedov @ 2012-12-10 21:33 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, ehabkost, afaerber
v2:
* Naming changes:
- s/visit_type_uint_suffixed_int/visit_type_suffixed_int/
- use 'suffix_factor' instead of 'unit'
* Added documentation to visit_type_suffixed_int()
* Fixed errp check.
* Style fixes
Reference to previous version dicussion:
http://lists.gnu.org/archive/html/qemu-devel/2012-12/msg00758.html
Git tree for testing:
https://github.com/imammedo/qemu/tree/type_suffixed_int_v2
Igor Mammedov (2):
add visitor for parsing int[KMGT] input string
target-i386: use visit_type_suffixed_int() to parse tsc_freq property
value
qapi/qapi-dealloc-visitor.c | 8 ++++++++
qapi/qapi-visit-core.c | 35 +++++++++++++++++++++++++++++++++++
qapi/qapi-visit-core.h | 4 ++++
qapi/string-input-visitor.c | 25 +++++++++++++++++++++++++
target-i386/cpu.c | 3 ++-
5 files changed, 74 insertions(+), 1 deletion(-)
--
1.7.11.7
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 1/2] add visitor for parsing int[KMGT] input string
2012-12-10 21:33 [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer Igor Mammedov
@ 2012-12-10 21:33 ` Igor Mammedov
2012-12-12 18:16 ` Eduardo Habkost
2012-12-10 21:33 ` [Qemu-devel] [PATCH 2/2] target-i386: use visit_type_suffixed_int() to parse tsc_freq property value Igor Mammedov
2012-12-23 20:34 ` [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer Anthony Liguori
2 siblings, 1 reply; 10+ messages in thread
From: Igor Mammedov @ 2012-12-10 21:33 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, ehabkost, afaerber
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 <imammedo@redhat.com>
---
v3:
- Fix errp check. Spotted-By: Andreas Färber <afaerber@suse.de>
- 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(+)
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 75214e7..09c3b01 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -143,6 +143,13 @@ static void qapi_dealloc_type_enum(Visitor *v, int *obj, const char *strings[],
{
}
+static void qapi_dealloc_type_suffixed_int(Visitor *v, int64_t *obj,
+ const char *name,
+ const int suffix_factor,
+ Error **errp)
+{
+}
+
Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
{
return &v->visitor;
@@ -170,6 +177,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
v->visitor.type_str = qapi_dealloc_type_str;
v->visitor.type_number = qapi_dealloc_type_number;
v->visitor.type_size = qapi_dealloc_type_size;
+ v->visitor.type_suffixed_int = qapi_dealloc_type_suffixed_int;
QTAILQ_INIT(&v->stack);
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 7a82b63..9f6b8ae 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -311,3 +311,38 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
g_free(enum_str);
*obj = value;
}
+
+/**
+ * visit_type_suffixed_int:
+ * @v: visitor used for accesing external representation of value
+ * @obj: object that stores internal value
+ * @name: name of the option
+ * @suffix_factor: multiplication factor of suffix 'K'
+ * @errp: object to store error code
+ *
+ * Converts option value represented by @v taking in account multiplication
+ * factor of a suffix character that might follow number.
+ * i.e. resulted *@obj = number * suffix.
+ *
+ * Where following suffixes are recognised:
+ * no suffix = 1
+ * K = @suffix_factor
+ * M = K * @suffix_factor
+ * G = M * @suffix_factor
+ * T = G * @suffix_factor
+ *
+ * In case @v doesn't provide type_suffixed_int() parser, convertion
+ * fall-backs to suffix-less type_int64() parser.
+ */
+void visit_type_suffixed_int(Visitor *v, int64_t *obj, const char *name,
+ const int suffix_factor, Error **errp)
+{
+ if (error_is_set(errp)) {
+ return;
+ }
+ if (v->type_suffixed_int) {
+ v->type_suffixed_int(v, obj, name, suffix_factor, errp);
+ } else {
+ visit_type_int64(v, obj, name, errp);
+ }
+}
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index 60aceda..05e9940 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -62,6 +62,8 @@ struct Visitor
void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
/* visit_type_size() falls back to (*type_uint64)() if type_size is unset */
void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
+ void (*type_suffixed_int)(Visitor *v, int64_t *obj, const char *name,
+ const int uffix_factor, Error **errp);
};
void visit_start_handle(Visitor *v, void **obj, const char *kind,
@@ -91,5 +93,7 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
+void visit_type_suffixed_int(Visitor *v, int64_t *obj, const char *name,
+ const int suffix_factor, Error **errp);
#endif
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 497eb9a..9641bce 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -110,6 +110,30 @@ static void parse_start_optional(Visitor *v, bool *present,
*present = true;
}
+static void parse_type_suffixed_int(Visitor *v, int64_t *obj, const char *name,
+ const int suffix_factor, Error **errp)
+{
+ StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+ char *endp = (char *) siv->string;
+ long long val = 0;
+
+ if (siv->string) {
+ /* TODO: presently strtosz_suffix_unit() is limited to [0..INT64_MAX]
+ * it should be fixed to handle [INT64_MIN..INT64_MAX],
+ * to cover whole int64_t range
+ */
+ val = strtosz_suffix_unit(siv->string, &endp,
+ STRTOSZ_DEFSUFFIX_B, suffix_factor);
+ }
+ if (!siv->string || val == -1 || *endp) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
+ "a value representable as a non-negative int64");
+ return;
+ }
+
+ *obj = val;
+}
+
Visitor *string_input_get_visitor(StringInputVisitor *v)
{
return &v->visitor;
@@ -132,6 +156,7 @@ StringInputVisitor *string_input_visitor_new(const char *str)
v->visitor.type_str = parse_type_str;
v->visitor.type_number = parse_type_number;
v->visitor.start_optional = parse_start_optional;
+ v->visitor.type_suffixed_int = parse_type_suffixed_int;
v->string = str;
return v;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/2] target-i386: use visit_type_suffixed_int() to parse tsc_freq property value
2012-12-10 21:33 [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer Igor Mammedov
2012-12-10 21:33 ` [Qemu-devel] [PATCH 1/2] add visitor for parsing int[KMGT] input string Igor Mammedov
@ 2012-12-10 21:33 ` Igor Mammedov
2012-12-12 18:16 ` Eduardo Habkost
2012-12-23 20:34 ` [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer Anthony Liguori
2 siblings, 1 reply; 10+ messages in thread
From: Igor Mammedov @ 2012-12-10 21:33 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, ehabkost, afaerber
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
- s/visit_type_unit_suffixed_int/visit_type_suffixed_int/
v2:
- replace visit_type_freq() with visit_type_unit_suffixed_int()
in x86_cpuid_set_tsc_freq()
---
target-i386/cpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7be3ad8..18adff4 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1194,8 +1194,9 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
const int64_t min = 0;
const int64_t max = INT64_MAX;
int64_t value;
+ const int kHz_factor = 1000;
- visit_type_int(v, &value, name, errp);
+ visit_type_suffixed_int(v, &value, name, kHz_factor, errp);
if (error_is_set(errp)) {
return;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] add visitor for parsing int[KMGT] input string
2012-12-10 21:33 ` [Qemu-devel] [PATCH 1/2] add visitor for parsing int[KMGT] input string Igor Mammedov
@ 2012-12-12 18:16 ` Eduardo Habkost
2012-12-14 13:20 ` Igor Mammedov
0 siblings, 1 reply; 10+ messages in thread
From: Eduardo Habkost @ 2012-12-12 18:16 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, afaerber, mdroth
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 <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
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
'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 <afaerber@suse.de>
> - 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] target-i386: use visit_type_suffixed_int() to parse tsc_freq property value
2012-12-10 21:33 ` [Qemu-devel] [PATCH 2/2] target-i386: use visit_type_suffixed_int() to parse tsc_freq property value Igor Mammedov
@ 2012-12-12 18:16 ` Eduardo Habkost
0 siblings, 0 replies; 10+ messages in thread
From: Eduardo Habkost @ 2012-12-12 18:16 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, afaerber, mdroth
On Mon, Dec 10, 2012 at 10:33:07PM +0100, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Like I said in the previous patch, maybe it would be nice to have a
visit_type_frequency() helper, to hide the specific details/parameters
required to parse frequency values.
> ---
> v3:
> - s/visit_type_unit_suffixed_int/visit_type_suffixed_int/
> v2:
> - replace visit_type_freq() with visit_type_unit_suffixed_int()
> in x86_cpuid_set_tsc_freq()
> ---
> target-i386/cpu.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 7be3ad8..18adff4 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1194,8 +1194,9 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
> const int64_t min = 0;
> const int64_t max = INT64_MAX;
> int64_t value;
> + const int kHz_factor = 1000;
>
> - visit_type_int(v, &value, name, errp);
> + visit_type_suffixed_int(v, &value, name, kHz_factor, errp);
> if (error_is_set(errp)) {
> return;
> }
> --
> 1.7.11.7
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] add visitor for parsing int[KMGT] input string
2012-12-12 18:16 ` Eduardo Habkost
@ 2012-12-14 13:20 ` Igor Mammedov
2012-12-14 18:18 ` Eduardo Habkost
0 siblings, 1 reply; 10+ messages in thread
From: Igor Mammedov @ 2012-12-14 13:20 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: mdroth, qemu-devel, afaerber
On Wed, 12 Dec 2012 16:16:42 -0200
Eduardo Habkost <ehabkost@redhat.com> 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 <imammedo@redhat.com>
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>
>
> 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 <afaerber@suse.de>
> > - 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] add visitor for parsing int[KMGT] input string
2012-12-14 13:20 ` Igor Mammedov
@ 2012-12-14 18:18 ` Eduardo Habkost
0 siblings, 0 replies; 10+ messages in thread
From: Eduardo Habkost @ 2012-12-14 18:18 UTC (permalink / raw)
To: Igor Mammedov; +Cc: mdroth, qemu-devel, afaerber
On Fri, Dec 14, 2012 at 02:20:48PM +0100, Igor Mammedov wrote:
> On Wed, 12 Dec 2012 16:16:42 -0200
> Eduardo Habkost <ehabkost@redhat.com> 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 <imammedo@redhat.com>
> >
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> >
> >
> > 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.
The above make sense, but I wasn't worrying about how it's done
internally when calling/configuring the parsing code. My only point is
about the type definitions in QAPI. I believe that having this in QAPI:
'type':'frequency'
'type':'size'
looks better than:
'type': 'suffixed_int', 'suffix_factor': 1000, 'unit': 'Hz'
'type': 'suffixed_int', 'suffix_factor': 1024, 'unit': 'B'
or, even worse:
'type': 'suffixed_int', 'suffix_table': { 'K': 1000, 'KHz': 1000, 'M': 1000000, 'MHz': 10000000, ... }
'type': 'suffixed_int', 'suffix_table': { 'K': 1024, 'KB': 1024, 'M': 1048576, 'MB': 1048576, ... }
or:
'type': 'suffixed_int', 'suffix_table': { 'K': 1000, 'M': 1000000, ... }, 'unit': 'Hz'
'type': 'suffixed_int', 'suffix_table': { 'K': 1024, 'M': 1048576, ... }, 'unit': 'B'
>
> > '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 <afaerber@suse.de>
> > > - 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
--
Eduardo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer
2012-12-10 21:33 [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer Igor Mammedov
2012-12-10 21:33 ` [Qemu-devel] [PATCH 1/2] add visitor for parsing int[KMGT] input string Igor Mammedov
2012-12-10 21:33 ` [Qemu-devel] [PATCH 2/2] target-i386: use visit_type_suffixed_int() to parse tsc_freq property value Igor Mammedov
@ 2012-12-23 20:34 ` Anthony Liguori
2012-12-24 14:03 ` Igor Mammedov
2012-12-26 12:51 ` Eduardo Habkost
2 siblings, 2 replies; 10+ messages in thread
From: Anthony Liguori @ 2012-12-23 20:34 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: mdroth, afaerber, ehabkost
Igor Mammedov <imammedo@redhat.com> writes:
> v2:
> * Naming changes:
> - s/visit_type_uint_suffixed_int/visit_type_suffixed_int/
> - use 'suffix_factor' instead of 'unit'
> * Added documentation to visit_type_suffixed_int()
> * Fixed errp check.
> * Style fixes
This is not how visitors are supposed to be used.
Just treat tsc_freq as a string property and parse it in the setter.
Regards,
Anthony Liguori
>
> Reference to previous version dicussion:
> http://lists.gnu.org/archive/html/qemu-devel/2012-12/msg00758.html
>
> Git tree for testing:
> https://github.com/imammedo/qemu/tree/type_suffixed_int_v2
>
> Igor Mammedov (2):
> add visitor for parsing int[KMGT] input string
> target-i386: use visit_type_suffixed_int() to parse tsc_freq property
> value
>
> qapi/qapi-dealloc-visitor.c | 8 ++++++++
> qapi/qapi-visit-core.c | 35 +++++++++++++++++++++++++++++++++++
> qapi/qapi-visit-core.h | 4 ++++
> qapi/string-input-visitor.c | 25 +++++++++++++++++++++++++
> target-i386/cpu.c | 3 ++-
> 5 files changed, 74 insertions(+), 1 deletion(-)
>
> --
> 1.7.11.7
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer
2012-12-23 20:34 ` [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer Anthony Liguori
@ 2012-12-24 14:03 ` Igor Mammedov
2012-12-26 12:51 ` Eduardo Habkost
1 sibling, 0 replies; 10+ messages in thread
From: Igor Mammedov @ 2012-12-24 14:03 UTC (permalink / raw)
To: Anthony Liguori; +Cc: afaerber, qemu-devel, ehabkost, mdroth
On Sun, 23 Dec 2012 14:34:21 -0600
Anthony Liguori <anthony@codemonkey.ws> wrote:
> Igor Mammedov <imammedo@redhat.com> writes:
>
> > v2:
> > * Naming changes:
> > - s/visit_type_uint_suffixed_int/visit_type_suffixed_int/
> > - use 'suffix_factor' instead of 'unit'
> > * Added documentation to visit_type_suffixed_int()
> > * Fixed errp check.
> > * Style fixes
>
> This is not how visitors are supposed to be used.
We were trying to generalize and make code more reusable.
Later could be used as a common base for specialized size and Hz visitors.
> Just treat tsc_freq as a string property and parse it in the setter.
Would http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg03835.html
be better or you suggest just abandon generalization idea and parse it
locally in target-i386/cpu.c?
>
> Regards,
>
> Anthony Liguori
>
> >
> > Reference to previous version dicussion:
> > http://lists.gnu.org/archive/html/qemu-devel/2012-12/msg00758.html
> >
> > Git tree for testing:
> > https://github.com/imammedo/qemu/tree/type_suffixed_int_v2
> >
> > Igor Mammedov (2):
> > add visitor for parsing int[KMGT] input string
> > target-i386: use visit_type_suffixed_int() to parse tsc_freq property
> > value
> >
> > qapi/qapi-dealloc-visitor.c | 8 ++++++++
> > qapi/qapi-visit-core.c | 35 +++++++++++++++++++++++++++++++++++
> > qapi/qapi-visit-core.h | 4 ++++
> > qapi/string-input-visitor.c | 25 +++++++++++++++++++++++++
> > target-i386/cpu.c | 3 ++-
> > 5 files changed, 74 insertions(+), 1 deletion(-)
> >
> > --
> > 1.7.11.7
--
Regards,
Igor
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer
2012-12-23 20:34 ` [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer Anthony Liguori
2012-12-24 14:03 ` Igor Mammedov
@ 2012-12-26 12:51 ` Eduardo Habkost
1 sibling, 0 replies; 10+ messages in thread
From: Eduardo Habkost @ 2012-12-26 12:51 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Igor Mammedov, qemu-devel, afaerber, mdroth
On Sun, Dec 23, 2012 at 02:34:21PM -0600, Anthony Liguori wrote:
> Igor Mammedov <imammedo@redhat.com> writes:
>
> > v2:
> > * Naming changes:
> > - s/visit_type_uint_suffixed_int/visit_type_suffixed_int/
> > - use 'suffix_factor' instead of 'unit'
> > * Added documentation to visit_type_suffixed_int()
> > * Fixed errp check.
> > * Style fixes
>
> This is not how visitors are supposed to be used.
>
> Just treat tsc_freq as a string property and parse it in the setter.
So, why visit_type_size() does exist? Should we work to eliminate it? If
not, why is it different from a "frequency" type?
--
Eduardo
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-12-26 12:53 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-10 21:33 [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer Igor Mammedov
2012-12-10 21:33 ` [Qemu-devel] [PATCH 1/2] add visitor for parsing int[KMGT] input string Igor Mammedov
2012-12-12 18:16 ` Eduardo Habkost
2012-12-14 13:20 ` Igor Mammedov
2012-12-14 18:18 ` Eduardo Habkost
2012-12-10 21:33 ` [Qemu-devel] [PATCH 2/2] target-i386: use visit_type_suffixed_int() to parse tsc_freq property value Igor Mammedov
2012-12-12 18:16 ` Eduardo Habkost
2012-12-23 20:34 ` [Qemu-devel] [PATCH 0/2 v2] introduce visitor for parsing suffixed integer Anthony Liguori
2012-12-24 14:03 ` Igor Mammedov
2012-12-26 12:51 ` Eduardo Habkost
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.