From e909fb2179724b5619213520ca46cc31b4e6713b Mon Sep 17 00:00:00 2001
From: Robin Haberkorn <robin.haberkorn@googlemail.com>
Date: Thu, 20 Nov 2014 05:08:19 +0100
Subject: Throw error when a macro terminates while a local q-reg is edited.

This is only a problem if the macro created the local Q-Register table
(i.e. not when called with ":M") but resulted in segfaults.
Since we do not want to save in a Q-Reg whether it is local
(and that wouldn't suffice anyway), we do it in the Q-Register table
cleanup. The corresponding QRegisterTable::clear() must be called
explicitly, since the RBTree::clear() called on destruction does not
and cannot throw errors.
If QRegisterTable::clear() has been called successfully, the default object
destructor will not do much. If it has thrown an error, the destructor
will clean up the remaining Q-Registers.
---
 src/parser.cpp     | 40 ++++++++++++++++++++++++++--------------
 src/qregisters.cpp | 24 ++++++++++++++++++++++++
 src/qregisters.h   |  4 +++-
 3 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/src/parser.cpp b/src/parser.cpp
index 0b11721..35b6add 100644
--- a/src/parser.cpp
+++ b/src/parser.cpp
@@ -146,25 +146,37 @@ Execute::macro(const gchar *macro, bool locals)
 	try {
 		step(macro, strlen(macro));
 
-		if (Goto::skip_label) {
-			Error error("Label \"%s\" not found",
-			            Goto::skip_label);
-			error.pos = strlen(macro);
-			String::get_coord(macro, error.pos,
-					  error.line, error.column);
-			throw error;
-		}
+		/*
+		 * Subsequent errors must still be
+		 * attached to this macro invocation.
+		 */
+		try {
+			if (Goto::skip_label)
+				throw Error("Label \"%s\" not found",
+				            Goto::skip_label);
+
+			if (States::current != &States::start)
+				/*
+				 * can only happen if we returned because
+				 * of macro end
+				 */
+				throw Error("Unterminated command");
 
-		if (States::current != &States::start) {
-			Error error("Unterminated command");
 			/*
-			 * can only happen if we returned because
-			 * of macro end
+			 * This handles the problem of Q-Registers
+			 * local to the macro invocation being edited
+			 * when the macro terminates.
+			 * QRegisterTable::clear() throws an error
+			 * if this happens and the Q-Reg editing
+			 * is undone.
 			 */
+			if (locals)
+				QRegisters::locals->clear();
+		} catch (Error &error) {
 			error.pos = strlen(macro);
 			String::get_coord(macro, error.pos,
-			                  error.line, error.column);
-			throw error;
+					  error.line, error.column);
+			throw; /* forward */
 		}
 	} catch (...) {
 		g_free(Goto::skip_label);
diff --git a/src/qregisters.cpp b/src/qregisters.cpp
index fdb804b..223a707 100644
--- a/src/qregisters.cpp
+++ b/src/qregisters.cpp
@@ -275,6 +275,30 @@ QRegisterTable::edit(QRegister *reg)
 	QRegisters::current = reg;
 }
 
+/*
+ * This is similar to RBTree::clear() but
+ * has the advantage that we can check whether some
+ * register is currently edited.
+ * Since this is not a destructor, we can throw
+ * errors.
+ * Therefore this method should be called before
+ * a (local) QRegisterTable is deleted.
+ */
+void
+QRegisterTable::clear(void)
+{
+	QRegister *cur;
+
+	while ((cur = (QRegister *)min())) {
+		if (cur == QRegisters::current)
+			throw Error("Currently edited Q-Register \"%s\" "
+			            "cannot be discarded", cur->name);
+
+		remove(cur);
+		delete cur;
+	}
+}
+
 void
 QRegisterStack::UndoTokenPush::run(void)
 {
diff --git a/src/qregisters.h b/src/qregisters.h
index efd802d..cf2c644 100644
--- a/src/qregisters.h
+++ b/src/qregisters.h
@@ -165,7 +165,7 @@ public:
 	void edit(void);
 };
 
-class QRegisterTable : public RBTree {
+class QRegisterTable : private RBTree {
 	class UndoTokenRemove : public UndoToken {
 		QRegisterTable *table;
 		QRegister *reg;
@@ -236,6 +236,8 @@ public:
 		edit(reg);
 		return reg;
 	}
+
+	void clear(void);
 };
 
 class QRegisterStack {
-- 
cgit v1.2.3