[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Part_combine_iterator::derived_mark: don't abort marking prematurely
From: |
dak |
Subject: |
Re: Part_combine_iterator::derived_mark: don't abort marking prematurely. (issue 6057044) |
Date: |
Wed, 18 Apr 2012 04:39:20 +0000 |
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 ());
}
- Re: Part_combine_iterator::derived_mark: don't abort marking prematurely. (issue 6057044),
dak <=