qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 03/23] memory: merge adjacent segments of a sing


From: Avi Kivity
Subject: Re: [Qemu-devel] [PATCH 03/23] memory: merge adjacent segments of a single memory region
Date: Tue, 26 Jul 2011 12:55:23 +0300
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.18) Gecko/20110621 Fedora/3.1.11-1.fc15 Thunderbird/3.1.11

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]