Am 27.05.2013 05:20, schrieb Michael Roth:
> With the introduction of native list types, we now have types such as
> int64List where the 'value' field is not a pointer, but the actual
> 64-bit value.
>
> On 32-bit architectures, this can lead to situations where 'next' field
> offset in GenericList does not correspond to the 'next' field in the
> types that we cast to GenericList when using the visit_next_list()
> interface, causing issues when we attempt to traverse linked list
> structures of these types.
>
> To fix this, pad the 'value' field of GenericList and other
> schema-defined/native *List types out to 64-bits.
>
> This is less memory-efficient for 32-bit architectures, but allows us to
> continue to rely on list-handling interfaces that target GenericList to
> simply visitor implementations.
>
> In the future we can improve efficiency by defaulting to using native C
> array backends to handle list of non-pointer types, which would be more
> memory efficient in itself and allow us to roll back this change.
>
> Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com>
> ---
> include/qapi/visitor.h | 5 ++++-
> scripts/qapi-types.py | 10 ++++++++--
> tests/test-qmp-output-visitor.c | 5 ++++-
> 3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index 1fef18c..28c21d8 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -18,7 +18,10 @@
>
> typedef struct GenericList
> {
> - void *value;
> + union {
> + void *value;
> + uint64_t padding;
> + };
> struct GenericList *next;
> } GenericList;
>
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index fd42d71..ddcfed9 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -22,7 +22,10 @@ def generate_fwd_struct(name, members, builtin_type=False):
>
> typedef struct %(name)sList
> {
> - %(type)s value;
> + union {
> + %(type)s value;
> + uint64_t padding;
> + };
> struct %(name)sList *next;
> } %(name)sList;
> ''',
> @@ -35,7 +38,10 @@ typedef struct %(name)s %(name)s;
>
> typedef struct %(name)sList
> {
> - %(name)s *value;
> + union {
> + %(name)s *value;
> + uint64_t padding;
> + };
> struct %(name)sList *next;
> } %(name)sList;
> ''',
> diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
> index 0942a41..b2fa9a7 100644
> --- a/tests/test-qmp-output-visitor.c
> +++ b/tests/test-qmp-output-visitor.c
> @@ -295,7 +295,10 @@ static void
> test_visitor_out_struct_errors(TestOutputVisitorData *data,
>
> typedef struct TestStructList
> {
> - TestStruct *value;
> + union {
> + TestStruct *value;
> + uint64_t padding;
> + };
> struct TestStructList *next;
> } TestStructList;
>
Looks good. Would reordering of value, next work, too
(without memory overhead for 32 bit systems)?
typedef struct GenericList
{
struct GenericList *next;
void *value;
} GenericList;
typedef struct %(name)sList
{
struct %(name)sList *next;
%(type)s value;
} %(name)sList;
...
It looks like memory allocation (g_malloc0) for GenericList
was also wrong in the old code (this is fixed with your patch).
