Re: [Qemu-devel] [PATCH 5/9] qapi_sized_buffer

[Stefan Berger <stefanb@linux.vnet.ibm.com>]

On 03/14/2013 11:11 AM, mdroth wrote:
> On Thu, Mar 14, 2013 at 10:51:49AM -0400, Stefan Berger wrote:
>> On 03/14/2013 10:28 AM, mdroth wrote:
>>> On Thu, Mar 14, 2013 at 09:39:14AM -0400, Stefan Berger wrote:
>>>> On 03/14/2013 08:18 AM, mdroth wrote:
>>>>> On Wed, Mar 13, 2013 at 09:48:11PM -0400, Stefan Berger wrote:
>>>>>> On 03/13/2013 07:18 PM, mdroth wrote:
>>>>>>> On Wed, Mar 13, 2013 at 06:00:24PM -0400, Stefan Berger wrote:
>>>>>>>> On 03/13/2013 04:52 PM, mdroth wrote:
>>>>>>>>
>>>>>>> Visitors don't have any knowledge of the data structures they're visiting
>>>>>>> outside of what we tell them via the visit_*() API.
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>> For example, a visitor for a 16-element array of:
>>>>>>>
>>>>>>> typedef struct ComplexType {
>>>>>>>      int32_t foo;
>>>>>>>      char *bar;
>>>>>>> } ComplexType;
>>>>>>>
>>>>>>> would look something like:
>>>>>>>
>>>>>>> visit_start_carray(v, ...); // instruct visitor how to calculate offsets
>>>>>>> for (i = 0; i < 16; i++) {
>>>>>>>      visit_type_ComplexType(v, ...) // instruct visitor how to handle elem
>>>>>>>      visit_next_carray(v, ...); // instruct visitor to move to next offset
>>>>>>> }
>>>>>>> visit_end_carray(v, ...); // instruct visitor to finalize array
>>>>>> Given this example above, I think we will need the sized buffer. The
>>>>>> sized buffer targets  binary arrays and their encoding. If I was to
>>>>>> encode an 'unsigned char[n]' (e.g., n=200) using n, or n/2 or n/4
>>>>>> loops like above breaking it apart in u8, u16 or u32 respectively I
>>>>>> think this would 'not bed good' also considering the 2 bytes for tag
>>>>>> and length being added by ASN.1 for every such datatype
>>>>>> (u8,u16,u32). The sized buffer allows you to for example take a
>>>>>> memory page and write it out in one chunk adding a few bytes of
>>>>>> ASN.1 'decoration' around the actual data.
>>>>> You could do it with this interface as well actually. The Visitor will
>>>>> need to maintain some internal state to differentiate what it does with
>>>>> subsequent visit_type*/visit_next_carray/visit_end_carry. There's no
>>>>> reason it couldn't also track the elem size so it could tag a buffer
>>>>> "en masse" when visit_end_carray() gets called.
>>>> It depends on what you pass into visit_start_carray. In your case if
>>>> you pass in ComplexType you would pass in a sizeof(ComplexType) for
>>>> the size of each element presumably. The problem is now you havechar *foo, a string pointer, hanging off of this structure. How
>>>> would you handle that? Serializing ComplexType's foo and pointer
>>>> obviously won't do it.
>>> Why not?  visit_type_ComplexType() knows how to deal with
>>> the individual fields, including the string pointer. I'm not sure
>>> what's at issue here.
>>>
>>> In this case the handling for ComplexType would look something like:
>>>
>>> visit_type_Complex:
>>>      visit_start_struct
>>>      visit_type_uin32 //foo
>>>      visit_type_str //bar
>>>      visit_end_struct
>>>
>>> Granted, strings are easier to deal with. If char * was instead a plain
>>> old uint8_t*, we'd need a nested call to start_carray for each element.
>>> in this case it would look something like:
>>>
>>> visit_type_Complex:
>>>      visit_start_struct
>>>      visit_type_uin32 //foo field
>>>      visit_start_carray //bar field
>>>      for (i = 0; i < len_of_bar; i++):
>>>          visit_type_uint8
>>>          visit_next_carray
>>>      visit_end_carray
>> You really want to create a separate element for each element in
>> this potentially large binary array? I guess it depends on the
>> underlying data, but this has the potential of generating a lot of
>> control code around each such byte... As said, for ASN.1 encoding,
>> each such byte would be decorated with a tag and a length value,
>> consuming 2 more bytes per byte.
> I addressed this earlier. Your visitor doesn't have tag each
> element: if it know it's handling an array (because we told it via
> start_carray()), it can buffer them internally and tag the array en
> masse when end_carray() is issued.

If we were to do this using carray on an array of structs of the 
following type

struct SimpleStruct {
     uint8_t a;
     uint8_t b;
     uint32_t c;
}

then the serialization of a and b would be buffered and flushed once the 
32bit output visitor (or any other than uint8_t output visitor) would be 
called? Now this does makes the implementation a lot more difficult.

   Stefan