Re: [Qemu-devel] [PATCH 1/3] vmstate: introduce calc_size VMStateField

[Mitsyanko Igor <i.mitsyanko@samsung.com>]

On 12/27/2011 05:10 PM, Andreas Färber wrote:
> Am 27.12.2011 09:11, schrieb Mitsyanko Igor:
>> On 12/26/2011 07:20 PM, Peter Maydell wrote:
>>> On 26 December 2011 10:03, Mitsyanko Igor<i.mitsyanko@samsung.com>
>>> wrote:
>>>> diff --git a/hw/hw.h b/hw/hw.h
>>>> index efa04d1..8ce4475 100644
>>>> --- a/hw/hw.h
>>>> +++ b/hw/hw.h
>>>> @@ -303,9 +303,9 @@ enum VMStateFlags {
>>>>       VMS_ARRAY_OF_POINTER = 0x040,
>>>>       VMS_VARRAY_UINT16    = 0x080,  /* Array with size in uint16_t
>>>> field */
>>>>       VMS_VBUFFER          = 0x100,  /* Buffer with size in int32_t
>>>> field */
>>>> -    VMS_MULTIPLY         = 0x200,  /* multiply "size" field by
>>>> field_size */
>>>> -    VMS_VARRAY_UINT8     = 0x400,  /* Array with size in uint8_t
>>>> field*/
>>>> -    VMS_VARRAY_UINT32    = 0x800,  /* Array with size in uint32_t
>>>> field*/
>>>> +    VMS_CALC_SIZE        = 0x200,  /* calculate size of dynamic
>>>> buffer */
>>>> +    VMS_VARRAY_UINT8     = 0x400,  /* Array with size in uint8_t
>>>> field */
>>>> +    VMS_VARRAY_UINT32    = 0x800,  /* Array with size in uint32_t
>>>> field */
>>>
>>> These unrelated whitespace fixes are confusing -- please drop them.
>> QEMU wiki here http://wiki.qemu.org/Contribute/SubmitAPatch states that
>> it's ok :)
>>> It's OK to fix coding style issues in the immediate area (few lines)>
>> of the lines you're changing.)
>
> It's not really a Coding Style issue though (just an aesthetic one),
> what's meant by the quote is to fix braces around the lines you touch.
>
> I'd suggest to put it in a separate preceding patch, then it gets fixed
> and we can still better see what you're changing here.

If v2 of this patch will be accepted and we will drop VMS_MULTIPLY, I
could add required spaces together with changing values given to 
VMS_VARRAY_UINT8 and VMS_VARRAY_UINT32 flags. Or this can be done by 
Paolo Bonzini in his "split hw/hw.h" patch, I cc him just in case he 
wouldn't mind to do this.

> (I'm in need of a mechanism like this for AHCI so please cc Juan and me
> on v2 of this patch.)
>
> Andreas
>

I'm also considering to change local size variable in 
vmstate_load_state() and vmstate_save_state() to size_t, it makes no 
sense to convert size_t field->size to intermediate int type just to 
pass it again as size_t to get() function.

-- 
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com