From: Georg Baum <baum@lyx.org>
Date: Fri, 14 Nov 2014 20:30:42 +0000 (+0100)
Subject: Fix recursive math macro crash (bug #9140)
X-Git-Tag: 2.2.0alpha1~1542
X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=0f2069b8a5967aac1b4f841214294f1fe21d2cad;p=features.git

Fix recursive math macro crash (bug #9140)

This is different from bug #8999, since in this case a new macro instance is
created. You still get a TeX capacity exceeded error if you try to typeset the
exported document, but this is the same as for bug #8999 and better than a
crash.
---

diff --git a/src/mathed/MacroTable.cpp b/src/mathed/MacroTable.cpp
index 6daa2a8aed..4af30d4c54 100644
--- a/src/mathed/MacroTable.cpp
+++ b/src/mathed/MacroTable.cpp
@@ -62,7 +62,7 @@ MacroData::MacroData(Buffer * buf, MathMacroTemplate const & macro)
 }
 
 
-void MacroData::expand(vector<MathData> const & args, MathData & to) const
+bool MacroData::expand(vector<MathData> const & args, MathData & to) const
 {
 	updateData();
 
@@ -70,10 +70,10 @@ void MacroData::expand(vector<MathData> const & args, MathData & to) const
 	static InsetMathSqrt inset(0);
 	inset.setBuffer(const_cast<Buffer &>(*buffer_));
 
-	// FIXME UNICODE
-	asArray(display_.empty() ? definition_ : display_, inset.cell(0));
+	docstring const & definition(display_.empty() ? definition_ : display_);
+	asArray(definition, inset.cell(0));
 	//lyxerr << "MathData::expand: args: " << args << endl;
-	//lyxerr << "MathData::expand: ar: " << inset.cell(0) << endl;
+	//LYXERR0("MathData::expand: ar: " << inset.cell(0));
 	for (DocIterator it = doc_iterator_begin(buffer_, &inset); it; it.forwardChar()) {
 		if (!it.nextInset())
 			continue;
@@ -87,8 +87,12 @@ void MacroData::expand(vector<MathData> const & args, MathData & to) const
 			it.cell().insert(it.pos(), args[n - 1]);
 		}
 	}
-	//lyxerr << "MathData::expand: res: " << inset.cell(0) << endl;
+	//LYXERR0("MathData::expand: res: " << inset.cell(0));
 	to = inset.cell(0);
+	// If the result is equal to the definition then we either have a
+	// recursive loop, or the definition did not contain any macro in the
+	// first place.
+	return asString(to) != definition;
 }
 
 
diff --git a/src/mathed/MacroTable.h b/src/mathed/MacroTable.h
index 6f8201f29e..0f9f60dc41 100644
--- a/src/mathed/MacroTable.h
+++ b/src/mathed/MacroTable.h
@@ -52,7 +52,8 @@ public:
 	size_t numargs() const { updateData(); return numargs_; }
 	/// replace #1,#2,... by given MathAtom 0,1,.., _including_ the possible
 	/// optional argument
-	void expand(std::vector<MathData> const & from, MathData & to) const;
+	/// \return whether anything was expanded
+	bool expand(std::vector<MathData> const & from, MathData & to) const;
 	/// number of optional arguments
 	size_t optionals() const;
 	///
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index f4150f30f8..941e29aee2 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -331,6 +331,7 @@ private:
 void MathMacro::updateRepresentation(Cursor * cur, MacroContext const & mc,
 		UpdateType utype)
 {
+	// block recursive calls (bug 8999)
 	if (isUpdating_)
 		return;
 
@@ -364,9 +365,15 @@ void MathMacro::updateRepresentation(Cursor * cur, MacroContext const & mc,
 		values[i].insert(0, MathAtom(proxy));
 	}
 	// expanding macro with the values
-	macro_->expand(values, expanded_.cell(0));
-	if (utype == OutputUpdate && !expanded_.cell(0).empty())
-		expanded_.cell(0).updateMacros(cur, mc, utype);
+	// Only update the argument macros if anything was expanded, otherwise
+	// we would get an endless loop (bug 9140). UpdateLocker does not work
+	// in this case, since MacroData::expand() creates new MathMacro
+	// objects, so this would be a different recursion path than the one
+	// protected by UpdateLocker.
+	if (macro_->expand(values, expanded_.cell(0))) {
+		if (utype == OutputUpdate && !expanded_.cell(0).empty())
+			expanded_.cell(0).updateMacros(cur, mc, utype);
+	}
 	// get definition for list edit mode
 	docstring const & display = macro_->display();
 	asArray(display.empty() ? macro_->definition() : display, definition_);