From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:57050)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <avi@redhat.com>) id 1QleML-00011n-LM
	for qemu-devel@nongnu.org; Tue, 26 Jul 2011 05:55:30 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <avi@redhat.com>) id 1QleMK-0007XY-NP
	for qemu-devel@nongnu.org; Tue, 26 Jul 2011 05:55:29 -0400
Received: from mx1.redhat.com ([209.132.183.28]:9950)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <avi@redhat.com>) id 1QleMK-0007XR-GF
	for qemu-devel@nongnu.org; Tue, 26 Jul 2011 05:55:28 -0400
Message-ID: <4E2E8F0B.9090402@redhat.com>
Date: Tue, 26 Jul 2011 12:55:23 +0300
From: Avi Kivity <avi@redhat.com>
MIME-Version: 1.0
References: <1311602584-23409-1-git-send-email-avi@redhat.com>
	<1311602584-23409-4-git-send-email-avi@redhat.com>
	<4E2DBA75.5010701@codemonkey.ws>
In-Reply-To: <4E2DBA75.5010701@codemonkey.ws>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 03/23] memory: merge adjacent segments of a
 single memory region
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: Anthony Liguori <anthony@codemonkey.ws>
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org

On 07/25/2011 09:48 PM, Anthony Liguori wrote:
>> +/* Attempt to simplify a view by merging ajacent ranges */
>> +static void flatview_simplify(FlatView *view)
>> +{
>> +    unsigned i;
>> +    FlatRange *r1, *r2;
>> +
>> +    for (i = 0; i + 1<  view->nr; ++i) {
>> +        r1 =&view->ranges[i];
>> +        r2 =&view->ranges[i+1];
>> +        if (addrrange_end(r1->addr) == r2->addr.start
>> +&&  r1->mr == r2->mr
>> +&&  r1->offset_in_region + r1->addr.size == r2->offset_in_region
>> +&&  r1->dirty_log_mask == r2->dirty_log_mask) {
>> +            r1->addr.size += r2->addr.size;
>> +            memmove(r2, r2 + 1, (view->nr - (i + 2)) * sizeof(*r2));
>> +            --view->nr;
>> +            --i;
>> +        }
>
>
> The --i is pretty subtle.  Moving the index variable backwards in a 
> conditional in a for loop is pretty evil :-)  I started writing up why 
> this was wrong until I noticed that.
>
> I think the following would be more straight forward:
>
> i = 0;
> while (i + 1 < view->nr) {
>    int begin = i, end = i + 1;
>
>    while (matches(&view->ranges[begin], &view->ranges[end])) {
>       end++;
>    }
>
>    memmove(...)
> }
>

Right; updated to something along these lines.

-- 
error compiling committee.c: too many arguments to function