"Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> * Juan Quintela (quint...@redhat.com) wrote:
>> Signed-off-by: Juan Quintela <quint...@redhat.com>
>> +static void obj_versioned_copy(void *arg1, void *arg2)
>> {
>> - QEMUFile *fsave = open_test_file(true);
>> - uint8_t buf[] = {
>> - 0, 0, 0, 10, /* a */
>> - 0, 0, 0, 30, /* c */
>> - 0, 0, 0, 0, 0, 0, 0, 40, /* d */
>> - QEMU_VM_EOF, /* just to ensure we won't get EOF reported
>> prematurely */
>> - };
>> - qemu_put_buffer(fsave, buf, sizeof(buf));
>> - qemu_fclose(fsave);
>> -
>> - QEMUFile *loading = open_test_file(false);
>> - TestStruct obj = { .b = 200, .e = 500, .f = 600 };
>> - vmstate_load_state(loading, &vmstate_versioned, &obj, 1);
>> - g_assert(!qemu_file_get_error(loading));
>> - g_assert_cmpint(obj.a, ==, 10);
>> - g_assert_cmpint(obj.b, ==, 200);
>> - g_assert_cmpint(obj.c, ==, 30);
>> - g_assert_cmpint(obj.d, ==, 40);
>> - g_assert_cmpint(obj.e, ==, 500);
>> - g_assert_cmpint(obj.f, ==, 600);
>> - qemu_fclose(loading);
>> + TestVersioned *target = arg1;
>> + TestVersioned *source = arg2;
>> +
>> + target->a = source->a;
>> + target->b = source->b;
>> + target->c = source->c;
>> + target->d = source->d;
>> + target->e = source->e;
>> + target->f = source->f;
>> + target->skip_c_e = source->skip_c_e;
>
> Why's that not simply *target = *source?
To be able to only copy some of the fields, depending on what we want to
test O:-)
>> + TestVersioned obj, obj_clone;
>> +
>> + memset(&obj, 0, sizeof(obj));
>> + save_vmstate(&vmstate_simple_versioned, &obj_versioned);
>> +
>> + compare_vmstate(wire_simple_v2, sizeof(wire_simple_v2));
>> +
>> + SUCCESS(load_vmstate(&vmstate_simple_versioned, &obj, &obj_clone,
>> + obj_versioned_copy, 2, wire_simple_v2,
>> + sizeof(wire_simple_v2)));
>> +
>> +#define FIELD_EQUAL(name) g_assert_cmpint(obj.name, ==,
>> obj_versioned.name)
>> +#define FIELD_NOT_EQUAL(name) \
>> + g_assert_cmpint(obj.name, !=, obj_versioned.name)
>
> Given that macro is shared with the next few functions it would be seem
> to declare it outside of the function.
>
It is not always the same (see the whole series otherwise). I ended
finding easier to do it this way. I ended defining it the 1st time that
I needed it. That is coherent with the whole series, but I can change
it (don't really matter).
>> + memset(&obj, 0, sizeof(obj));
>> + obj_versioned.skip_c_e = false;
>> + save_vmstate(&vmstate_simple_skipping, &obj_versioned);
>> +
>> + compare_vmstate(wire_simple_no_skip, sizeof(wire_simple_no_skip));
>> +
>> + /* We abuse the fact that f has a 0x00 value in the right position */
>
> A bit nasty.
Any better ideas?
>> + compare_vmstate(wire_simple_skip, sizeof(wire_simple_skip));
>> +
>> + /* We abuse the fact that f has a 0x00 value in the right position */
>> + SUCCESS(load_vmstate(&vmstate_simple_skipping, &obj, &obj_clone,
>> + obj_versioned_copy, 1, wire_simple_skip,
>> + sizeof(wire_simple_skip) - 8));
>> +
>> + FIELD_EQUAL(skip_c_e);
>> + FIELD_EQUAL(a);
>> + FIELD_EQUAL(b);
>> + FIELD_NOT_EQUAL(c);
>> + FIELD_EQUAL(d);
>> + FIELD_NOT_EQUAL(e);
>> + FIELD_NOT_EQUAL(f);
>> +
>> + SUCCESS(load_vmstate(&vmstate_simple_skipping, &obj, &obj_clone,
>> + obj_versioned_copy, 2, wire_simple_skip,
>> + sizeof(wire_simple_skip)));
>> +
>> + FIELD_EQUAL(skip_c_e);
>> + FIELD_EQUAL(a);
>> + FIELD_EQUAL(b);
>> + FIELD_NOT_EQUAL(c);
>> + FIELD_EQUAL(d);
>> + FIELD_NOT_EQUAL(e);
>> + FIELD_EQUAL(f);
>> }
>
> Couldn't those functions just be merged and take a flag?
>I think it is clear this way, but that is the same to me.
FIELD_EQUAL(a)
FIELD_NOT_EQUAL(a)
vs
COMPARE_FIELD(a, true)
COMPARE_FILED(b, false)
For me, I don't need to go to the definition of the macro in the 1st
case, and I do on the second one.
Or do you mean anything different?
Thanks, Juan.
