Looks good to me. I'd guess that it works because you prevent the compiler optimizations from doing whatever optimization it did on dynamic_cast.
This is a reasonable change, IMO, and it solves the problem, so I'm in favor of pushing it. Carl On 9/4/10 10:40 AM, "[email protected]" <[email protected]> wrote: > Reviewers: , > > Message: > Hi everybody, > > This patch appears to fix the bus error/segfault reported in issue 818, > but it's unclear to me why it works. > > Please review. > > Thanks, > Neil > > Description: > Fix #818. > > Use unsmob_engraver ()/unsmob_performer () instead of > explicit casting in translator-group.cc when filtering > translators added via \with { } > > * lily/include/performer.hh, lily/performer.hh > > add unsmob_performer () > > * lily/translator-group.cc (filter_engravers/filter_performers): > > use unsmob_* () instead of dynamic_cast when checking > translator is of correct class to remove > > Please review this at http://codereview.appspot.com/2128043/ > > Affected files: > M lily/include/performer.hh > M lily/performer.cc > M lily/translator-group.cc > > > Index: lily/include/performer.hh > diff --git a/lily/include/performer.hh b/lily/include/performer.hh > index > 11a5c32c8a59bdf4976d231afcbac05b49cc288e..0b0497e322b2fdacf9ab32fe09fde3bc7842 > f98a > 100644 > --- a/lily/include/performer.hh > +++ b/lily/include/performer.hh > @@ -40,5 +40,7 @@ protected: > virtual void create_audio_elements (); > }; > > +Performer *unsmob_performer (SCM perf); > + > #endif /* PERFORMER_HH */ > > Index: lily/performer.cc > diff --git a/lily/performer.cc b/lily/performer.cc > index > c177f9e975e2760df30700a2470095ccc00c8313..60a0519d42b31336abaa8b1bff32022a4630 > 5581 > 100644 > --- a/lily/performer.cc > +++ b/lily/performer.cc > @@ -19,6 +19,7 @@ > */ > > #include "context.hh" > +#include "performer.hh" > #include "performer-group.hh" > #include "warn.hh" > > @@ -48,3 +49,9 @@ Performer::announce_element (Audio_element_info i) > > get_daddy_performer ()->announce_element (i); > } > + > +Performer * > +unsmob_performer (SCM perf) > +{ > + return dynamic_cast<Performer *> (unsmob_translator (perf)); > +} > Index: lily/translator-group.cc > diff --git a/lily/translator-group.cc b/lily/translator-group.cc > index > 84a97bf52e206bdf980ab6bae228fabac470cbab..6093e4780f8c951a33bb3c148971f2cc5469 > 48c4 > 100644 > --- a/lily/translator-group.cc > +++ b/lily/translator-group.cc > @@ -23,11 +23,13 @@ > #include "context-def.hh" > #include "context.hh" > #include "dispatcher.hh" > +#include "engraver.hh" > #include "engraver-group.hh" > #include "international.hh" > #include "main.hh" > #include "music.hh" > #include "output-def.hh" > +#include "performer.hh" > #include "performer-group.hh" > #include "scheme-engraver.hh" > #include "scm-hash.hh" > @@ -90,7 +92,7 @@ filter_performers (SCM ell) > SCM *tail = ℓ > for (SCM p = ell; scm_is_pair (p); p = scm_cdr (p)) > { > - if (dynamic_cast<Performer *> (unsmob_translator (scm_car (*tail)))) > + if (unsmob_performer (scm_car (*tail))) > *tail = scm_cdr (*tail); > else > tail = SCM_CDRLOC (*tail); > @@ -104,7 +106,7 @@ filter_engravers (SCM ell) > SCM *tail = ℓ > for (SCM p = ell; scm_is_pair (p); p = scm_cdr (p)) > { > - if (dynamic_cast<Engraver *> (unsmob_translator (scm_car (*tail)))) > + if (unsmob_engraver (scm_car (*tail))) > *tail = scm_cdr (*tail); > else > tail = SCM_CDRLOC (*tail); > > > > _______________________________________________ > lilypond-devel mailing list > [email protected] > http://lists.gnu.org/mailman/listinfo/lilypond-devel _______________________________________________ lilypond-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/lilypond-devel
