lilypond-devel
[Top][All Lists]
Advanced

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

Re: Fix #1427. (issue3319041)


From: n . puttock
Subject: Re: Fix #1427. (issue3319041)
Date: Thu, 25 Nov 2010 00:21:21 +0000

Reviewers: Reinhold,

Message:
On 2010/11/25 00:16:22, Reinhold wrote:
I can't comment on the code itself (from a first look it makes sense,
but I
haven't dug too deep). What is missing, though, is a regtest...

I've held off doing a regtest until somebody can confirm this is the
right course of action, though if you insist... :)



Description:
Fix #1427.

The copy constructor for Book assumes the list of objects to clone are
scores, which is unsafe since a \book(part) block can also contain
markup and
page markers.

* lily/book.cc (copy constructor):

  treat entries in scores_ separately, cloning Score and Page_marker
objects

* lily/include/page-marker.hh, lily/page-marker.cc:

  add copy constructor

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

Affected files:
  M lily/book.cc
  M lily/include/page-marker.hh
  M lily/page-marker.cc


Index: lily/book.cc
diff --git a/lily/book.cc b/lily/book.cc
index efd26d44ebb6fe452b3cfe8a9af3cf4360b12db6..66567aa79a98a02d123fa1e5c5088030578a5463 100644
--- a/lily/book.cc
+++ b/lily/book.cc
@@ -67,15 +67,22 @@ Book::Book (Book const &s)
   header_ = ly_make_module (false);
   if (ly_is_module (s.header_))
     ly_module_copy (header_, s.header_);
-
   SCM *t = &scores_;
   for (SCM p = s.scores_; scm_is_pair (p); p = scm_cdr (p))
     {
-      Score *newscore = unsmob_score (scm_car (p))->clone ();
+      SCM entry = scm_car (p);
+
+      if (Score *newscore = unsmob_score (entry))
+        *t = scm_cons (newscore->clone ()->unprotect (), SCM_EOL);
+      else if (Page_marker *marker = unsmob_page_marker (entry))
+        *t = scm_cons (marker->clone ()->unprotect (), SCM_EOL);
+      else
+        {
+          /* This entry is a markup list */
+          *t = scm_cons (entry, SCM_EOL);
+        }

-      *t = scm_cons (newscore->self_scm (), SCM_EOL);
       t = SCM_CDRLOC (*t);
-      newscore->unprotect ();
     }

   t = &bookparts_;
Index: lily/include/page-marker.hh
diff --git a/lily/include/page-marker.hh b/lily/include/page-marker.hh
index c8b7e57ddc60b2da0a1195b2587c981ecb72d207..e11d9e6eb2962f7823050bc054801ae0008ff6c6 100644
--- a/lily/include/page-marker.hh
+++ b/lily/include/page-marker.hh
@@ -21,6 +21,7 @@
 #define PAGE_MARKER_HH

 #include "smobs.hh"
+#include "virtual-methods.hh"

 class Page_marker
 {
@@ -32,6 +33,8 @@ class Page_marker

 public:
   Page_marker ();
+  Page_marker (Page_marker const &);
+  VIRTUAL_COPY_CONSTRUCTOR (Page_marker, Page_marker);

   void set_permission (SCM symbol, SCM permission);
   void set_label (SCM label);
Index: lily/page-marker.cc
diff --git a/lily/page-marker.cc b/lily/page-marker.cc
index fc7b30524d0994dd26157ad3436fc2bb614bab1c..6cf0ae08ff803db17dcd2e7d96f33282df21b872 100644
--- a/lily/page-marker.cc
+++ b/lily/page-marker.cc
@@ -28,6 +28,14 @@ Page_marker::Page_marker ()
   smobify_self ();
 }

+Page_marker::Page_marker (Page_marker const &src)
+{
+  symbol_ = src.symbol_;
+  permission_ = src.permission_;
+  label_ = src.label_;
+  smobify_self ();
+}
+
 Page_marker::~Page_marker ()
 {
 }





reply via email to

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