[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 02/23] qapi: Require int64/uint64 implementat
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v6 02/23] qapi: Require int64/uint64 implementation |
Date: |
Fri, 27 Nov 2015 13:05:05 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> Now that all visitors supply both type_int64() and type_uint64()
> callbacks, we can drop the redundant type_int() callback (the
> public interface visit_type_int() remains, but calls into
> type_int64() under the hood).
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v6: new patch, but stems from v5 23/46
> ---
> include/qapi/visitor-impl.h | 1 -
> qapi/opts-visitor.c | 1 -
> qapi/qapi-dealloc-visitor.c | 1 -
> qapi/qapi-visit-core.c | 50
> ++++++++++++++------------------------------
> qapi/qmp-input-visitor.c | 1 -
> qapi/qmp-output-visitor.c | 1 -
> qapi/string-input-visitor.c | 1 -
> qapi/string-output-visitor.c | 1 -
> 8 files changed, 16 insertions(+), 41 deletions(-)
>
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 44a21b7..70326e0 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -36,7 +36,6 @@ struct Visitor
> void (*get_next_type)(Visitor *v, QType *type, bool promote_int,
> const char *name, Error **errp);
>
> - void (*type_int)(Visitor *v, int64_t *obj, const char *name, Error
> **errp);
> void (*type_bool)(Visitor *v, bool *obj, const char *name, Error **errp);
> void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
> void (*type_number)(Visitor *v, double *obj, const char *name,
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index bc2b976..8104e42 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -522,7 +522,6 @@ opts_visitor_new(const QemuOpts *opts)
> */
> ov->visitor.type_enum = &input_type_enum;
>
> - ov->visitor.type_int = &opts_type_int64;
> ov->visitor.type_int64 = &opts_type_int64;
> ov->visitor.type_uint64 = &opts_type_uint64;
> ov->visitor.type_size = &opts_type_size;
> diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> index 5d1b2e6..5133d37 100644
> --- a/qapi/qapi-dealloc-visitor.c
> +++ b/qapi/qapi-dealloc-visitor.c
> @@ -220,7 +220,6 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
> v->visitor.next_list = qapi_dealloc_next_list;
> v->visitor.end_list = qapi_dealloc_end_list;
> v->visitor.type_enum = qapi_dealloc_type_enum;
> - v->visitor.type_int = qapi_dealloc_type_int64;
> v->visitor.type_int64 = qapi_dealloc_type_int64;
> v->visitor.type_uint64 = qapi_dealloc_type_uint64;
> v->visitor.type_bool = qapi_dealloc_type_bool;
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 6d63e40..88bed9c 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -97,19 +97,19 @@ void visit_type_enum(Visitor *v, int *obj, const char *
> const strings[],
>
> void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
> {
> - v->type_int(v, obj, name, errp);
> + v->type_int64(v, obj, name, errp);
> }
>
> void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error
> **errp)
> {
> - int64_t value;
> + uint64_t value;
>
> if (v->type_uint8) {
> v->type_uint8(v, obj, name, errp);
> } else {
> value = *obj;
> - v->type_int(v, &value, name, errp);
> - if (value < 0 || value > UINT8_MAX) {
> + v->type_uint64(v, &value, name, errp);
> + if (value > UINT8_MAX) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> name ? name : "null", "uint8_t");
> return;
Note that this relies on value being in range after type_uint64() fails.
If it isn't, we call error_setg() with non-null *errp.
Two solutions:
1. Stipulate that type_uint64() & friends leave value alone on error.
Works, because its initial value *obj is in range.
2. Avoid using value on error. A clean way to do this:
Error *err = NULL;
value = *obj;
v->type_uint64(v, &value, name, &err);
if (err) {
error_propagate(errp, err);
return;
}
if (value < 0 || value > UINT8_MAX) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
name ? name : "null", "uint8_t");
return;
}
*obj = value;
More boilerplate. If we pick this solution, we'll want a separate
PATCH 1.5 cleaning up the preexisting instances.
> @@ -120,14 +120,14 @@ void visit_type_uint8(Visitor *v, uint8_t *obj, const
> char *name, Error **errp)
>
> void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error
> **errp)
> {
> - int64_t value;
> + uint64_t value;
>
> if (v->type_uint16) {
> v->type_uint16(v, obj, name, errp);
> } else {
> value = *obj;
> - v->type_int(v, &value, name, errp);
> - if (value < 0 || value > UINT16_MAX) {
> + v->type_uint64(v, &value, name, errp);
> + if (value > UINT16_MAX) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> name ? name : "null", "uint16_t");
> return;
Good. The fewer signed-unsigned conversions, the better.
[...]
- [Qemu-devel] [PATCH v6 00/23] qapi visitor cleanups (post-introspection cleanups subset E), Eric Blake, 2015/11/25
- [Qemu-devel] [PATCH v6 04/23] qapi: Don't cast Enum* to int*, Eric Blake, 2015/11/25
- [Qemu-devel] [PATCH v6 03/23] qapi: Consolidate visitor integer callbacks, Eric Blake, 2015/11/25
- [Qemu-devel] [PATCH v6 02/23] qapi: Require int64/uint64 implementation, Eric Blake, 2015/11/25
- Re: [Qemu-devel] [PATCH v6 02/23] qapi: Require int64/uint64 implementation,
Markus Armbruster <=
- [Qemu-devel] [PATCH v6 06/23] qapi: Don't abuse stack to track qmp-output root, Eric Blake, 2015/11/25
- [Qemu-devel] [PATCH v6 05/23] qmp: Fix reference-counting of qnull on empty output visit, Eric Blake, 2015/11/25
- [Qemu-devel] [PATCH v6 01/23] qapi: Make all visitors supply int64/uint64 callbacks, Eric Blake, 2015/11/25
- [Qemu-devel] [PATCH v6 09/23] hmp: Improve use of qapi visitor, Eric Blake, 2015/11/25
- [Qemu-devel] [PATCH v6 11/23] ppc: Improve use of qapi visitors, Eric Blake, 2015/11/25
- [Qemu-devel] [PATCH v6 14/23] qapi: Fix command with named empty argument type, Eric Blake, 2015/11/25
- [Qemu-devel] [PATCH v6 16/23] qapi: Track all failures between visit_start/stop, Eric Blake, 2015/11/25
- [Qemu-devel] [PATCH v6 10/23] vl: Improve use of qapi visitor, Eric Blake, 2015/11/25