Re: [Qemu-devel] [RFC 2/6] OptsVisitor: introduce list modes for interval flattening

[Laszlo Ersek <lersek@redhat.com>]

On 07/18/13 16:56, Paolo Bonzini wrote:
> Il 18/07/2013 15:59, Laszlo Ersek ha scritto:
>> The new modes are equal-rank, exclusive sub-modes of LM_IN_PROGRESS. Teach
>> opts_next_list(), opts_type_int() and opts_type_uint64() to handle them.
> 
> Perhaps you could use a bitmap then:
> 
>      LM_NONE = 0
>      LM_STARTED = 1
>      LM_IN_PROGRESS = 2
>      LM_SIGNED_INTERVAL = LM_IN_PROGRESS | 4
>      LM_UNSIGNED_INTERVAL = LM_IN_PROGRESS | 8
> 
> I think the only change would be that this hunk:
> 
>> @@ -211,7 +238,10 @@ opts_end_list(Visitor *v, Error **errp)
>>  {
>>      OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
>>  
>> -    assert(ov->list_mode == LM_STARTED || ov->list_mode == LM_IN_PROGRESS);
>> +    assert(ov->list_mode == LM_STARTED ||
>> +           ov->list_mode == LM_IN_PROGRESS ||
>> +           ov->list_mode == LM_SIGNED_INTERVAL ||
>> +           ov->list_mode == LM_UNSIGNED_INTERVAL);
>>      ov->repeated_opts = NULL;
>>      ov->list_mode = LM_NONE;
>>  }
> 
> could be changed to
> 
> 	assert(ov->list_mode == LM_STARTED ||
>                (ov->list_mode & LM_IN_PROGRESS));

If you don't insist (please don't :)), I wouldn't like to do this.

(a) I wanted to represent and query each individual mode explicitly
(helps with code search),

(b) the "sub-mode" nature is a theoretical thing. It only applies to the
two interval modes. This small orthogonality is limited, it doesn't
cause "combinatorial explosion" (I didn't have to double the states or
such). Most importantly, I specifically wanted one state in general to
exclude any other state. Bitmaps beget thinking about the meaning of
arbitrary variations, like 1|4, 0|2 etc; I intended to prevent such
thoughts right in the type.

Thanks
Laszlo