Revision tags: v7.0.0, v7.0.0-rc4, v7.0.0-rc3, v7.0.0-rc2, v7.0.0-rc1, v7.0.0-rc0, v6.1.1, v6.2.0, v6.2.0-rc4, v6.2.0-rc3, v6.2.0-rc2, v6.2.0-rc1, v6.2.0-rc0, v6.0.1, v6.1.0, v6.1.0-rc4, v6.1.0-rc3, v6.1.0-rc2, v6.1.0-rc1, v6.1.0-rc0, v6.0.0, v6.0.0-rc5, v6.0.0-rc4, v6.0.0-rc3, v6.0.0-rc2, v6.0.0-rc1, v6.0.0-rc0, v5.2.0, v5.2.0-rc4, v5.2.0-rc3, v5.2.0-rc2, v5.2.0-rc1, v5.2.0-rc0, v5.0.1, v5.1.0, v5.1.0-rc3, v5.1.0-rc2, v5.1.0-rc1, v5.1.0-rc0 |
|
#
012d4c96 |
| 07-Jul-2020 |
Markus Armbruster <armbru@redhat.com> |
qapi: Make visitor functions taking Error ** return bool, not void
See recent commit "error: Document Error API usage rules" for rationale.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Revi
qapi: Make visitor functions taking Error ** return bool, not void
See recent commit "error: Document Error API usage rules" for rationale.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-18-armbru@redhat.com>
show more ...
|
Revision tags: v4.2.1, v5.0.0 |
|
#
8b7ce95b |
| 24-Apr-2020 |
Markus Armbruster <armbru@redhat.com> |
qapi: Fix Visitor contract for start_alternate()
The contract demands v->start_alternate() for input and dealloc visitors, but visit_start_alternate() actually requires it for input and clone visito
qapi: Fix Visitor contract for start_alternate()
The contract demands v->start_alternate() for input and dealloc visitors, but visit_start_alternate() actually requires it for input and clone visitors. Fix the contract, and delete superfluous qapi_dealloc_start_alternate().
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-8-armbru@redhat.com>
show more ...
|
Revision tags: v5.0.0-rc4, v5.0.0-rc3, v5.0.0-rc2, v5.0.0-rc1, v5.0.0-rc0, v4.2.0, v4.2.0-rc5, v4.2.0-rc4, v4.2.0-rc3, v4.2.0-rc2, v4.1.1, v4.2.0-rc1, v4.2.0-rc0, v4.0.1, v3.1.1.1, v4.1.0, v4.1.0-rc5 |
|
#
dc5e9ac7 |
| 12-Aug-2019 |
Markus Armbruster <armbru@redhat.com> |
Include qemu/queue.h slightly less
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Include qemu/queue.h slightly less
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20190812052359.30071-20-armbru@redhat.com>
show more ...
|
Revision tags: v4.1.0-rc4, v3.1.1, v4.1.0-rc3, v4.1.0-rc2, v4.1.0-rc1, v4.1.0-rc0 |
|
#
a8d25326 |
| 23-May-2019 |
Markus Armbruster <armbru@redhat.com> |
Include qemu-common.h exactly where needed
No header includes qemu-common.h after this commit, as prescribed by qemu-common.h's file comment.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Me
Include qemu-common.h exactly where needed
No header includes qemu-common.h after this commit, as prescribed by qemu-common.h's file comment.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20190523143508.25387-5-armbru@redhat.com> [Rebased with conflicts resolved automatically, except for include/hw/arm/xlnx-zynqmp.h hw/arm/nrf51_soc.c hw/arm/msf2-soc.c block/qcow2-refcount.c block/qcow2-cluster.c block/qcow2-cache.c target/arm/cpu.h target/lm32/cpu.h target/m68k/cpu.h target/mips/cpu.h target/moxie/cpu.h target/nios2/cpu.h target/openrisc/cpu.h target/riscv/cpu.h target/tilegx/cpu.h target/tricore/cpu.h target/unicore32/cpu.h target/xtensa/cpu.h; bsd-user/main.c and net/tap-bsd.c fixed up]
show more ...
|
Revision tags: v4.0.0, v4.0.0-rc4, v3.0.1, v4.0.0-rc3, v4.0.0-rc2, v4.0.0-rc1, v4.0.0-rc0, v3.1.0, v3.1.0-rc5, v3.1.0-rc4, v3.1.0-rc3, v3.1.0-rc2, v3.1.0-rc1, v3.1.0-rc0, v3.0.0, v3.0.0-rc4, v2.12.1, v3.0.0-rc3, v3.0.0-rc2, v3.0.0-rc1, v3.0.0-rc0, v2.11.2, v2.12.0 |
|
#
cb3e7f08 |
| 19-Apr-2018 |
Marc-André Lureau <marcandre.lureau@redhat.com> |
qobject: Replace qobject_incref/QINCREF qobject_decref/QDECREF
Now that we can safely call QOBJECT() on QObject * as well as its subtypes, we can have macros qobject_ref() / qobject_unref() that wor
qobject: Replace qobject_incref/QINCREF qobject_decref/QDECREF
Now that we can safely call QOBJECT() on QObject * as well as its subtypes, we can have macros qobject_ref() / qobject_unref() that work everywhere instead of having to use QINCREF() / QDECREF() for QObject and qobject_incref() / qobject_decref() for its subtypes.
The replacement is mechanical, except I broke a long line, and added a cast in monitor_qmp_cleanup_req_queue_locked(). Unlike qobject_decref(), qobject_unref() doesn't accept void *.
Note that the new macros evaluate their argument exactly once, thus no need to shout them.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20180419150145.24795-4-marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Rebased, semantic conflict resolved, commit message improved] Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
Revision tags: v2.12.0-rc4, v2.12.0-rc3, v2.12.0-rc2, v2.12.0-rc1, v2.12.0-rc0, v2.11.1 |
|
#
15280c36 |
| 01-Feb-2018 |
Markus Armbruster <armbru@redhat.com> |
qdict qlist: Make most helper macros functions
The macro expansions of qdict_put_TYPE() and qlist_append_TYPE() need qbool.h, qnull.h, qnum.h and qstring.h to compile. We include qnull.h and qnum.h
qdict qlist: Make most helper macros functions
The macro expansions of qdict_put_TYPE() and qlist_append_TYPE() need qbool.h, qnull.h, qnum.h and qstring.h to compile. We include qnull.h and qnum.h in the headers, but not qbool.h and qstring.h. Works, because we include those wherever the macros get used.
Open-coding these helpers is of dubious value. Turn them into functions and drop the includes from the headers.
This cleanup makes the number of objects depending on qapi/qmp/qnum.h from 4551 (out of 4743) to 46 in my "build everything" tree. For qapi/qmp/qnull.h, the number drops from 4552 to 21.
Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-10-armbru@redhat.com>
show more ...
|
#
6b673957 |
| 01-Feb-2018 |
Markus Armbruster <armbru@redhat.com> |
Eliminate qapi/qmp/types.h
qapi/qmp/types.h is a convenience header to include a number of qapi/qmp/ headers. Since we rarely need all of the headers qapi/qmp/types.h includes, we bypass it most of
Eliminate qapi/qmp/types.h
qapi/qmp/types.h is a convenience header to include a number of qapi/qmp/ headers. Since we rarely need all of the headers qapi/qmp/types.h includes, we bypass it most of the time. Most of the places that use it don't need all the headers, either.
Include the necessary headers directly, and drop qapi/qmp/types.h.
Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-9-armbru@redhat.com>
show more ...
|
Revision tags: v2.10.2, v2.11.0, v2.11.0-rc5, v2.11.0-rc4, v2.11.0-rc3, v2.11.0-rc2, v2.11.0-rc1, v2.11.0-rc0, v2.10.1, v2.9.1, v2.10.0, v2.10.0-rc4, v2.10.0-rc3, v2.10.0-rc2, v2.10.0-rc1, v2.10.0-rc0 |
|
#
d2f95f4d |
| 26-Jun-2017 |
Markus Armbruster <armbru@redhat.com> |
qapi: Use QNull for a more regular visit_type_null()
Make visit_type_null() take an @obj argument like its buddies. This helps keep the next commit simple.
Signed-off-by: Markus Armbruster <armbru
qapi: Use QNull for a more regular visit_type_null()
Make visit_type_null() take an @obj argument like its buddies. This helps keep the next commit simple.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
show more ...
|
#
60390d2d |
| 07-Jun-2017 |
Marc-André Lureau <marcandre.lureau@redhat.com> |
qapi: Remove visit_start_alternate() parameter promote_int
Before the previous commit, parameter promote_int = true made visit_start_alternate() with an input visitor avoid QTYPE_QINT variants and c
qapi: Remove visit_start_alternate() parameter promote_int
Before the previous commit, parameter promote_int = true made visit_start_alternate() with an input visitor avoid QTYPE_QINT variants and create QTYPE_QFLOAT variants instead. This was used where QTYPE_QINT variants were invalid.
The previous commit fused QTYPE_QINT with QTYPE_QFLOAT, rendering promote_int useless and unused.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20170607163635.17635-8-marcandre.lureau@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
Revision tags: v2.8.1.1, v2.9.0, v2.9.0-rc5, v2.9.0-rc4, v2.9.0-rc3, v2.8.1, v2.9.0-rc2, v2.9.0-rc1, v2.9.0-rc0, v2.7.1, v2.8.0, v2.8.0-rc4, v2.8.0-rc3, v2.8.0-rc2, v2.8.0-rc1, v2.8.0-rc0, v2.6.2, v2.7.0, v2.7.0-rc5, v2.7.0-rc4, v2.6.1, v2.7.0-rc3, v2.7.0-rc2, v2.7.0-rc1, v2.7.0-rc0 |
|
#
2c0ef9f4 |
| 09-Jun-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Add new visit_free() function
Making each visitor provide its own (awkwardly-named) FOO_cleanup() is unusual, when we can instead have a polymorphic visit_free() interface. Over the next few
qapi: Add new visit_free() function
Making each visitor provide its own (awkwardly-named) FOO_cleanup() is unusual, when we can instead have a polymorphic visit_free() interface. Over the next few patches, we can use the polymorphic functions to eliminate the need for a FOO_get_visitor() function for accessing specific visitor functionality, once everything can be accessed directly through the Visitor* interfaces.
The dealloc visitor is the first one converted to completely use the new entry point, since qapi_dealloc_visitor_cleanup() was the only reason that qapi_dealloc_get_visitor() existed, and only generated and testsuite code was even using it. With the new visit_free() entry point in place, we no longer need to expose the QapiDeallocVisitor subtype through qapi_dealloc_visitor_new(), and can get by with less generated code, with diffs that look like:
| void qapi_free_ACPIOSTInfo(ACPIOSTInfo *obj) | { |- QapiDeallocVisitor *qdv; | Visitor *v; | | if (!obj) { | return; | } | |- qdv = qapi_dealloc_visitor_new(); |- v = qapi_dealloc_get_visitor(qdv); |+ v = qapi_dealloc_visitor_new(); | visit_type_ACPIOSTInfo(v, NULL, &obj, NULL); |- qapi_dealloc_visitor_cleanup(qdv); |+ visit_free(v); |}
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1465490926-28625-5-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
1158bb2a |
| 09-Jun-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Add parameter to visit_end_*
Rather than making the dealloc visitor track of stack of pointers remembered during visit_start_* in order to free them during visit_end_*, it's a lot easier to ju
qapi: Add parameter to visit_end_*
Rather than making the dealloc visitor track of stack of pointers remembered during visit_start_* in order to free them during visit_end_*, it's a lot easier to just make all callers pass the same pointer to visit_end_*. The generated code has access to the same pointer, while all other users are doing virtual walks and can pass NULL. The dealloc visitor is then greatly simplified.
All three visit_end_*() functions intentionally take a void**, even though the visit_start_*() functions differ between void**, GenericList**, and GenericAlternate**. This is done for several reasons: when doing a virtual walk, passing NULL doesn't care what the type is, but when doing a generated walk, we already have to cast the caller's specific FOO* to call visit_start, while using void** lets us use visit_end without a cast. Also, an upcoming patch will add a clone visitor that wants to use the same implementation for all three visit_end callbacks, which is made easier if all three share the same signature.
For visitors with already track per-object state (the QMP visitors via a stack, and the string visitors which do not allow nesting), add an assertion that the caller is indeed passing the same pointer to paired calls.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1465490926-28625-4-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
Revision tags: v2.6.0, v2.5.1.1, v2.6.0-rc5, v2.6.0-rc4 |
|
#
d9f62dde |
| 28-Apr-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Simplify semantics of visit_next_list()
The semantics of the list visit are somewhat baroque, with the following pseudocode when FooList is used:
start() for (prev = head; cur = next(prev); p
qapi: Simplify semantics of visit_next_list()
The semantics of the list visit are somewhat baroque, with the following pseudocode when FooList is used:
start() for (prev = head; cur = next(prev); prev = &cur) { visit(&cur->value) }
Note that these semantics (advance before visit) requires that the first call to next() return the list head, while all other calls return the next element of the list; that is, every visitor implementation is required to track extra state to decide whether to return the input as-is, or to advance. It also requires an argument of 'GenericList **' to next(), solely because the first iteration might need to modify the caller's GenericList head, so that all other calls have to do a layer of dereferencing.
Thankfully, we only have two uses of list visits in the entire code base: one in spapr_drc (which completely avoids visit_next_list(), feeding in integers from a different source than uint8List), and one in qapi-visit.py. That is, all other list visitors are generated in qapi-visit.c, and share the same paradigm based on a qapi FooList type, so we can refactor how lists are laid out with minimal churn among clients.
We can greatly simplify things by hoisting the special case into the start() routine, and flipping the order in the loop to visit before advance:
start(head) for (tail = *head; tail; tail = next(tail)) { visit(&tail->value) }
With the simpler semantics, visitors have less state to track, the argument to next() is reduced to 'GenericList *', and it also becomes obvious whether an input visitor is allocating a FooList during visit_start_list() (rather than the old way of not knowing if an allocation happened until the first visit_next_list()). As a minor drawback, we now allocate in two functions instead of one, and have to pass the size to both functions (unless we were to tweak the input visitors to cache the size to start_list for reuse during next_list, but that defeats the goal of less visitor state).
The signature of visit_start_list() is chosen to match visit_start_struct(), with the new parameters after 'name'.
The spapr_drc case is a virtual visit, done by passing NULL for list, similarly to how NULL is passed to visit_start_struct() when a qapi type is not used in those visits. It was easy to provide these semantics for qmp-output and dealloc visitors, and a bit harder for qmp-input (several prerequisite patches refactored things to make this patch straightforward). But it turned out that the string and opts visitors munge enough other state during visit_next_list() to make it easier to just document and require a GenericList visit for now; an assertion will remind us to adjust things if we need the semantics in the future.
Several pre-requisite cleanup patches made the reshuffling of the various visitors easier; particularly the qmp input visitor.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1461879932-9020-24-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
15c2f669 |
| 28-Apr-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Split visit_end_struct() into pieces
As mentioned in previous patches, we want to call visit_end_struct() functions unconditionally, so that visitors can release resources tied up since the ma
qapi: Split visit_end_struct() into pieces
As mentioned in previous patches, we want to call visit_end_struct() functions unconditionally, so that visitors can release resources tied up since the matching visit_start_struct() without also having to worry about error priority if more than one error occurs.
Even though error_propagate() can be safely used to ignore a second error during cleanup caused by a first error, it is simpler if the cleanup cannot set an error. So, split out the error checking portion (basically, input visitors checking for unvisited keys) into a new function visit_check_struct(), which can be safely skipped if any earlier errors are encountered, and leave the cleanup portion (which never fails, but must be called unconditionally if visit_start_struct() succeeded) in visit_end_struct().
Generated code in qapi-visit.c has diffs resembling:
|@@ -59,10 +59,12 @@ void visit_type_ACPIOSTInfo(Visitor *v, | goto out_obj; | } | visit_type_ACPIOSTInfo_members(v, obj, &err); |- error_propagate(errp, err); |- err = NULL; |+ if (err) { |+ goto out_obj; |+ } |+ visit_check_struct(v, &err); | out_obj: |- visit_end_struct(v, &err); |+ visit_end_struct(v); | out:
and in qapi-event.c:
@@ -47,7 +47,10 @@ void qapi_event_send_acpi_device_ost(ACP | goto out; | } | visit_type_q_obj_ACPI_DEVICE_OST_arg_members(v, ¶m, &err); |- visit_end_struct(v, err ? NULL : &err); |+ if (!err) { |+ visit_check_struct(v, &err); |+ } |+ visit_end_struct(v); | if (err) { | goto out;
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1461879932-9020-20-git-send-email-eblake@redhat.com> [Conflict with a doc fixup resolved] Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
3bc97fd5 |
| 28-Apr-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Add visit_type_null() visitor
Right now, qmp-output-visitor happens to produce a QNull result if nothing is actually visited between the creation of the visitor and the request for the resulti
qapi: Add visit_type_null() visitor
Right now, qmp-output-visitor happens to produce a QNull result if nothing is actually visited between the creation of the visitor and the request for the resulting QObject. A stronger protocol would require that a QMP output visit MUST visit something. But to still be able to produce a JSON 'null' output, we need a new visitor function that states our intentions. Yes, we could say that such a visit must go through visit_type_any(), but that feels clunky.
So this patch introduces the new visit_type_null() interface and its no-op interface in the dealloc visitor, and stubs in the qmp visitors (the next patch will finish the implementation). For the visitors that will not implement the callback, document the situation. The code in qapi-visit-core unconditionally dereferences the callback pointer, so that a segfault will inform a developer if they need to implement the callback for their choice of visitor.
Note that JSON has a primitive null type, with the single value null; likewise with the QNull type for QObject; but for QAPI, we just have the 'null' value without a null type. We may eventually want to add more support in QAPI for null (most likely, we'd use it via an alternate type that permits 'null' or an object); but we'll create that usage when we need it.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1461879932-9020-15-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
983f52d4 |
| 28-Apr-2016 |
Eric Blake <eblake@redhat.com> |
qapi-visit: Add visitor.type classification
We have three classes of QAPI visitors: input, output, and dealloc. Currently, all implementations of these visitors have one thing in common based on the
qapi-visit: Add visitor.type classification
We have three classes of QAPI visitors: input, output, and dealloc. Currently, all implementations of these visitors have one thing in common based on their visitor type: the implementation used for the visit_type_enum() callback. But since we plan to add more such common behavior, in relation to documenting and further refining the semantics, it makes more sense to have the visitor implementations advertise which class they belong to, so the common qapi-visit-core code can use that information in multiple places.
A later patch will better document the types of visitors directly in visitor.h.
For this patch, knowing the class of a visitor implementation lets us make input_type_enum() and output_type_enum() become static functions, by replacing the callback function Visitor.type_enum() with the simpler enum member Visitor.type. Share a common assertion in qapi-visit-core as part of the refactoring.
Move comments in opts-visitor.c to match the refactored layout.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1461879932-9020-2-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
Revision tags: v2.6.0-rc3, v2.6.0-rc2, v2.6.0-rc1, v2.6.0-rc0, v2.5.1 |
|
#
dbf11922 |
| 18-Feb-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Change visit_start_implicit_struct to visit_start_alternate
After recent changes, the only remaining use of visit_start_implicit_struct() is for allocating the space needed when visiting an al
qapi: Change visit_start_implicit_struct to visit_start_alternate
After recent changes, the only remaining use of visit_start_implicit_struct() is for allocating the space needed when visiting an alternate. Since the term 'implicit struct' is hard to explain, rename the function to its current usage. While at it, we can merge the functionality of visit_get_next_type() into the same function, making it more like visit_start_struct().
Generated code is now slightly smaller:
| { | Error *err = NULL; | |- visit_start_implicit_struct(v, (void**) obj, sizeof(BlockdevRef), &err); |+ visit_start_alternate(v, name, (GenericAlternate **)obj, sizeof(**obj), |+ true, &err); | if (err) { | goto out; | } |- visit_get_next_type(v, name, &(*obj)->type, true, &err); |- if (err) { |- goto out_obj; |- } | switch ((*obj)->type) { | case QTYPE_QDICT: | visit_start_struct(v, name, NULL, 0, &err); ... | } |-out_obj: |- visit_end_implicit_struct(v); |+ visit_end_alternate(v); | out: | error_propagate(errp, err); | }
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1455778109-6278-16-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
544a3731 |
| 18-Feb-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Don't box branches of flat unions
There's no reason to do two malloc's for a flat union; let's just inline the branch struct directly into the C union branch of the flat union.
Surprisingly,
qapi: Don't box branches of flat unions
There's no reason to do two malloc's for a flat union; let's just inline the branch struct directly into the C union branch of the flat union.
Surprisingly, fewer clients were actually using explicit references to the branch types in comparison to the number of flat unions thus modified.
This lets us reduce the hack in qapi-types:gen_variants() added in the previous patch; we no longer need to distinguish between alternates and flat unions.
The change to unboxed structs means that u.data (added in commit cee2dedb) is now coincident with random fields of each branch of the flat union, whereas beforehand it was only coincident with pointers (since all branches of a flat union have to be objects). Note that this was already the case for simple unions - but there we got lucky. Remember, visit_start_union() blindly returns true for all visitors except for the dealloc visitor, where it returns the value !!obj->u.data, and that this result then controls whether to proceed with the visit to the variant. Pre-patch, this meant that flat unions were testing whether the boxed pointer was still NULL, and thereby skipping visit_end_implicit_struct() and avoiding a NULL dereference if the pointer had not been allocated. The same was true for simple unions where the current branch had pointer type, except there we bypassed visit_type_FOO(). But for simple unions where the current branch had scalar type, the contents of that scalar meant that the decision to call visit_type_FOO() was data-dependent - the reason we got lucky there is that visit_type_FOO() for all scalar types in the dealloc visitor is a no-op (only the pointer variants had anything to free), so it did not matter whether the dealloc visit was skipped. But with this patch, we would risk leaking memory if we could skip a call to visit_type_FOO_fields() based solely on a data-dependent decision.
But notice: in the dealloc visitor, visit_type_FOO() already handles a NULL obj - it was only the visit_type_implicit_FOO() that was failing to check for NULL. And now that we have refactored things to have the branch be part of the parent struct, we no longer have a separate pointer that can be NULL in the first place. So we can just delete the call to visit_start_union() altogether, and blindly visit the branch type; there is no change in behavior except to the dealloc visitor, where we now unconditionally visit the branch, but where that visit is now always safe (for a flat union, we can no longer dereference NULL, and for a simple union, visit_type_FOO() was already safely handling NULL on pointer types).
Unfortunately, simple unions are not as easy to switch to unboxed layout; because we are special-casing the hidden implicit type with a single 'data' member, we really DO need to keep calling another layer of visit_start_struct(), with a second malloc; although there are some cleanups planned for simple unions in later patches.
visit_start_union() and gen_visit_implicit_struct() are now unused. Drop them.
Note that after this patch, the only remaining use of visit_start_implicit_struct() is for alternate types; the next patch will do further cleanup based on that fact.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1455778109-6278-14-git-send-email-eblake@redhat.com> [Dead code deletion squashed in, commit message updated accordingly] Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
e65d89bf |
| 18-Feb-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Adjust layout of FooList types
By sticking the next pointer first, we don't need a union with 64-bit padding for smaller types. On 32-bit platforms, this can reduce the size of uint8List from
qapi: Adjust layout of FooList types
By sticking the next pointer first, we don't need a union with 64-bit padding for smaller types. On 32-bit platforms, this can reduce the size of uint8List from 16 bytes (or 12, depending on whether 64-bit ints can tolerate 4-byte alignment) down to 8. It has no effect on 64-bit platforms (where alignment still dictates a 16-byte struct); but fewer anonymous unions is still a win in my book.
It requires visit_next_list() to gain a size parameter, to know what size element to allocate; comparable to the size parameter of visit_start_struct().
I debated about going one step further, to allow for fewer casts, by doing: typedef GenericList GenericList; struct GenericList { GenericList *next; }; struct FooList { GenericList base; Foo *value; }; so that you convert to 'GenericList *' by '&foolist->base', and back by 'container_of(generic, GenericList, base)' (as opposed to the existing '(GenericList *)foolist' and '(FooList *)generic'). But doing that would require hoisting the declaration of GenericList prior to inclusion of qapi-types.h, rather than its current spot in visitor.h; it also makes iteration a bit more verbose through 'foolist->base.next' instead of 'foolist->next'.
Note that for lists of objects, the 'value' payload is still hidden behind a boxed pointer. Someday, it would be nice to do:
struct FooList { FooList *next; Foo value; };
for one less level of malloc for each list element. This patch is a step in that direction (now that 'next' is no longer at a fixed non-zero offset within the struct, we can store more than just a pointer's-worth of data as the value payload), but the actual conversion would be a task for another series, as it will touch a lot of code.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1455778109-6278-10-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
08f9541d |
| 29-Jan-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Drop unused error argument for list and implicit struct
No backend was setting an error when ending the visit of a list or implicit struct, or when moving to the next list node. Make the call
qapi: Drop unused error argument for list and implicit struct
No backend was setting an error when ending the visit of a list or implicit struct, or when moving to the next list node. Make the callers a bit easier to follow by making this a part of the contract, and removing the errp argument - callers can then unconditionally end an object as part of cleanup without having to think about whether a second error is dominated by a first, because there is no second error.
A later patch will then tackle the larger task of splitting visit_end_struct(), which can indeed set an error.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1454075341-13658-24-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
337283df |
| 29-Jan-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Drop unused 'kind' for struct/enum visit
visit_start_struct() and visit_type_enum() had a 'kind' argument that was usually set to either the stringized version of the corresponding qapi type n
qapi: Drop unused 'kind' for struct/enum visit
visit_start_struct() and visit_type_enum() had a 'kind' argument that was usually set to either the stringized version of the corresponding qapi type name, or to NULL (although some clients didn't even get that right). But nothing ever used the argument. It's even hard to argue that it would be useful in a debugger, as a stack backtrace also tells which type is being visited.
Therefore, drop the 'kind' argument as dead.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <1454075341-13658-22-git-send-email-eblake@redhat.com> [Harmless rebase mistake cleaned up] Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
0b2a0d6b |
| 29-Jan-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Swap 'name' in visit_* callbacks to match public API
As explained in the previous patches, matching argument order of 'name, &value' to JSON's "name":value makes sense. However, while the las
qapi: Swap 'name' in visit_* callbacks to match public API
As explained in the previous patches, matching argument order of 'name, &value' to JSON's "name":value makes sense. However, while the last two patches were easy with Coccinelle, I ended up doing this one all by hand. Now all the visitor callbacks match the main interface.
The compiler is able to enforce that all clients match the changed interface in visitor-impl.h, even where two pointers are being swapped, because only one of the two pointers is const (if that were not the case, then C's looseness on treating 'char *' like 'void *' would have made review a bit harder).
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <1454075341-13658-21-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
f755dea7 |
| 29-Jan-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Make all visitors supply uint64 callbacks
Our qapi visitor contract supports multiple integer visitors, but left the type_uint64 visitor as optional (falling back on type_int64); which in turn
qapi: Make all visitors supply uint64 callbacks
Our qapi visitor contract supports multiple integer visitors, but left the type_uint64 visitor as optional (falling back on type_int64); which in turn can lead to awkward behavior with numbers larger than INT64_MAX (the user has to be aware of twos complement, and deal with negatives).
This patch does not address the disparity in handling large values as negatives. It merely moves the fallback from uint64 to int64 from the visitor core to the visitors, where the issue can actually be fixed, by implementing the missing type_uint64() callbacks on top of the respective type_int64() callbacks, and with a FIXME comment explaining why that's wrong.
With that done, we now have a type_uint64() callback in every driver, so we can make it mandatory from the core. And although the type_int64() callback can cover the entire valid range of type_uint{8,16,32} on valid user input, using type_uint64() to avoid mixed signedness makes more sense.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1454075341-13658-15-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
4c40314a |
| 29-Jan-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Prefer type_int64 over type_int in visitors
The qapi builtin type 'int' is basically shorthand for the type 'int64'. In fact, since no visitor was providing the optional type_int64() callback
qapi: Prefer type_int64 over type_int in visitors
The qapi builtin type 'int' is basically shorthand for the type 'int64'. In fact, since no visitor was providing the optional type_int64() callback, visit_type_int64() was just always falling back to type_int(), cementing the equivalence between the types.
However, some visitors are providing a type_uint64() callback. For purposes of code consistency, it is nicer if all visitors use the paired type_int64/type_uint64 names rather than the mismatched type_int/type_uint64. So this patch just renames the signed int callbacks in place, dropping the type_int() callback as redundant, and a later patch will focus on the unsigned int callbacks.
Add some FIXMEs to questionable reuse of errp in code touched by the rename, while at it (the reuse works as long as the callbacks don't modify value when setting an error, but it's not a good example to set) - a later patch will then fix those.
No change in functionality here, although further cleanups are in the pipeline.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1454075341-13658-14-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
4894b00b |
| 29-Jan-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Dealloc visitor does not need a type_size()
The intent of having the visitor type_size() callback differ from type_uint64() is to allow special handling for sizes; the visitor core gracefully
qapi: Dealloc visitor does not need a type_size()
The intent of having the visitor type_size() callback differ from type_uint64() is to allow special handling for sizes; the visitor core gracefully falls back to type_uint64() if there is no need for the distinction. Since the dealloc visitor does nothing for any of the int visits, drop the pointless size handler.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1454075341-13658-5-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
#
77577cb8 |
| 29-Jan-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Drop dead dealloc visitor variable
Commit 0b9d8542 added StackEntry.is_list_head, but forgot to delete the now-unused QapiDeallocVisitor.is_list_head.
Signed-off-by: Eric Blake <eblake@redhat
qapi: Drop dead dealloc visitor variable
Commit 0b9d8542 added StackEntry.is_list_head, but forgot to delete the now-unused QapiDeallocVisitor.is_list_head.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <1454075341-13658-4-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|