From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:51554)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lersek@redhat.com>) id 1UzqoN-00036C-SZ
	for qemu-devel@nongnu.org; Thu, 18 Jul 2013 12:12:15 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <lersek@redhat.com>) id 1UzqoJ-000512-2X
	for qemu-devel@nongnu.org; Thu, 18 Jul 2013 12:12:11 -0400
Received: from mx1.redhat.com ([209.132.183.28]:56716)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lersek@redhat.com>) id 1UzqXb-000680-Qk
	for qemu-devel@nongnu.org; Thu, 18 Jul 2013 11:54:52 -0400
Message-ID: <51E81056.8000004@redhat.com>
Date: Thu, 18 Jul 2013 17:57:10 +0200
From: Laszlo Ersek <lersek@redhat.com>
MIME-Version: 1.0
References: <1374155964-11278-1-git-send-email-lersek@redhat.com>
	<1374155964-11278-3-git-send-email-lersek@redhat.com>
	<51E8022A.7090008@redhat.com>
In-Reply-To: <51E8022A.7090008@redhat.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [RFC 2/6] OptsVisitor: introduce list modes for
	interval flattening
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, gaowanlong@cn.fujitsu.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