Re: Part_combine_iterator::derived_mark: don't abort marking prematurely. (issue 6057044)

[dak]

Reviewers: Graham Percival,

Message:
On 2012/04/17 20:21:14, Graham Percival wrote:
> LGTM and can be pushed right now.

> But for my general ignorance: why not just hard-code 4?  I suppose
that using
> the sizeof() trick makes it a bit safer in case somebody adds
something to ptrs,
> but is there any other reason?

No.  It is just a common C idiom.  Personally, I think it would make
more sense to just unfold the loop.  Takes fewer lines and is clearer.

I just kept with the original style.

For an array-based style, an automatic array makes little sense over
explicit code.  One could instead use a static array with
pointers-to-member (basically offsets), and then dereference them. In
that case, a 0 entry would indeed work as an ending mark.

But for 4 entries?

Description:
Part_combine_iterator::derived_mark: don't abort marking prematurely.

Please review this at http://codereview.appspot.com/6057044/

Affected files:
  M lily/part-combine-iterator.cc


Index: lily/part-combine-iterator.cc
diff --git a/lily/part-combine-iterator.cc b/lily/part-combine-iterator.cc
index  
eb1cca43e57092ed886f3166c6ae739d97c03970..5378aff4ecf405614aab7a9a978cca166193b90c  
100644
--- a/lily/part-combine-iterator.cc
+++ b/lily/part-combine-iterator.cc
@@ -165,10 +165,9 @@ Part_combine_iterator::derived_mark () const
     unisono_event_,
     mmrest_event_,
     solo_two_event_,
-    solo_one_event_,
-    0
+    solo_one_event_
   };
-  for (int i = 0; ptrs[i]; i++)
+  for (vsize i = 0; i < sizeof (ptrs)/sizeof (*ptrs); i++)
     if (ptrs[i])
       scm_gc_mark (ptrs[i]->self_scm ());
 }