Fix bug #9131 for development branch. There are two parts to the fix.

The firs tinvolves a thinko in BibTeXInfo::expandFormat. We were previously
counting passes through this routine, which means: one for every character,
more or less. So long strings would hit the "recursion limit". But what
we are worried about is an infinite loop caused by misues of macros, so that
is what we need to count.

This prevents the error we were previously getting, but it reveals a huge
slowdown when one tries to open a citation inset with a large nubmer of keys.
So we also limit the number of keys we try to process, and the length of the
string we try to display, when we are generating citation information.

I'm convinced that there is a deeper problem in how citation information is
generated (see the bug tracker for more info), but that will require major
surgery and a file format change
This commit is contained in:
Richard Heck 2014-05-23 10:59:12 -04:00
parent ba17d842b4
commit b992968a15
6 changed files with 53 additions and 28 deletions

View File

@ -488,7 +488,7 @@ docstring BibTeXInfo::expandFormat(docstring const & format,
// we'll remove characters from the front of fmt as we
// deal with them
while (!fmt.empty()) {
if (counter++ > max_passes) {
if (counter > max_passes) {
LYXERR0("Recursion limit reached while parsing `"
<< format << "'.");
return _("ERROR!");
@ -506,6 +506,7 @@ docstring BibTeXInfo::expandFormat(docstring const & format,
string const val =
buf.params().documentClass().getCiteMacro(engine_type, key);
fmt = from_utf8(val) + fmt.substr(1);
counter += 1;
continue;
} else if (key[0] == '_') {
// a translatable bit
@ -550,12 +551,15 @@ docstring BibTeXInfo::expandFormat(docstring const & format,
getValueForKey(optkey, buf, before, after, dialog, xref);
if (optkey == "next" && next)
ret << ifpart; // without expansion
else if (!val.empty())
ret << expandFormat(ifpart, xref, counter, buf,
else if (!val.empty()) {
int newcounter = 0;
ret << expandFormat(ifpart, xref, newcounter, buf,
before, after, dialog, next);
else if (!elsepart.empty())
ret << expandFormat(elsepart, xref, counter, buf,
} else if (!elsepart.empty()) {
int newcounter = 0;
ret << expandFormat(elsepart, xref, newcounter, buf,
before, after, dialog, next);
}
// fmt will have been shortened for us already
continue;
}
@ -878,10 +882,19 @@ docstring const BiblioInfo::getInfo(docstring const & key,
}
docstring const BiblioInfo::getLabel(vector<docstring> const & keys,
Buffer const & buf, string const & style, bool richtext,
docstring const & before, docstring const & after, docstring const & dialog) const
docstring const BiblioInfo::getLabel(vector<docstring> keys,
Buffer const & buf, string const & style, bool for_xhtml,
size_t max_size, docstring const & before, docstring const & after,
docstring const & dialog) const
{
// shorter makes no sense
LASSERT(max_size >= 16, max_size = 16);
// we can't display more than 10 of these, anyway
bool const too_many_keys = keys.size() > 10;
if (too_many_keys)
keys.resize(10);
CiteEngineType const engine_type = buf.params().citeEngineType();
DocumentClass const & dc = buf.params().documentClass();
docstring const & format = from_utf8(dc.getCiteFormat(engine_type, style, "cite"));
@ -903,8 +916,17 @@ docstring const BiblioInfo::getLabel(vector<docstring> const & keys,
xrefptr = &(xrefit->second);
}
}
ret = data.getLabel(xrefptr, buf, ret, richtext,
before, after, dialog, key+1 != ken);
ret = data.getLabel(xrefptr, buf, ret, for_xhtml,
before, after, dialog, key + 1 != ken);
}
if (ret.size() > max_size) {
ret.resize(max_size - 3);
ret += "...";
} else if (too_many_keys) {
if (ret.size() > max_size - 3)
ret.resize(max_size - 3);
ret += "...";
}
return ret;
}
@ -921,8 +943,8 @@ bool BiblioInfo::isBibtex(docstring const & key) const
vector<docstring> const BiblioInfo::getCiteStrings(
vector<docstring> const & keys, vector<CitationStyle> const & styles,
Buffer const & buf, bool richtext, docstring const & before,
docstring const & after, docstring const & dialog) const
Buffer const & buf, docstring const & before,
docstring const & after, docstring const & dialog, size_t max_size) const
{
if (empty())
return vector<docstring>();
@ -931,7 +953,7 @@ vector<docstring> const BiblioInfo::getCiteStrings(
vector<docstring> vec(styles.size());
for (size_t i = 0; i != vec.size(); ++i) {
style = styles[i].cmd;
vec[i] = getLabel(keys, buf, style, richtext, before, after, dialog);
vec[i] = getLabel(keys, buf, style, false, max_size, before, after, dialog);
}
return vec;

View File

@ -19,9 +19,9 @@
#include "Citation.h"
#include <vector>
#include <map>
#include <set>
#include <vector>
namespace lyx {
@ -67,7 +67,7 @@ public:
/// \return formatted BibTeX data for a citation label
docstring const getLabel(BibTeXInfo const * const xref,
Buffer const & buf, docstring const & format, bool richtext,
const docstring & before, const docstring & after,
const docstring & before, const docstring & after,
const docstring & dialog, bool next = false) const;
///
const_iterator find(docstring const & f) const { return bimap_.find(f); }
@ -203,9 +203,9 @@ public:
bool richtext = false) const;
/// \return formatted BibTeX data for citation labels.
/// Citation labels can have more than one key.
docstring const getLabel(std::vector<docstring> const & keys,
Buffer const & buf, std::string const & style, bool richtext,
docstring const & before, docstring const & after,
docstring const getLabel(std::vector<docstring> keys,
Buffer const & buf, std::string const & style, bool for_xhtml,
size_t max_size, docstring const & before, docstring const & after,
docstring const & dialog = docstring()) const;
/// Is this a reference from a bibtex database
/// or from a bibliography environment?
@ -214,9 +214,9 @@ public:
/// list of keys, using either numerical or author-year style depending
/// upon the active engine.
std::vector<docstring> const getCiteStrings(std::vector<docstring> const & keys,
std::vector<CitationStyle> const & styles, Buffer const & buf, bool richtext,
docstring const & before, docstring const & after,
docstring const & dialog = docstring()) const;
std::vector<CitationStyle> const & styles, Buffer const & buf,
docstring const & before, docstring const & after, docstring const & dialog,
size_t max_size) const;
/// Collects the cited entries from buf.
void collectCitedEntries(Buffer const & buf);
/// A list of BibTeX keys cited in the current document, sorted by

View File

@ -252,7 +252,8 @@ void GuiCitation::updateStyles(BiblioInfo const & bi)
if (!selectedLV->selectionModel()->selectedIndexes().empty())
curr = selectedLV->selectionModel()->selectedIndexes()[0].row();
QStringList sty = citationStyles(bi);
static const size_t max_length = 80;
QStringList sty = citationStyles(bi, max_length);
if (sty.isEmpty()) {
// some error
@ -590,7 +591,7 @@ void GuiCitation::findKey(BiblioInfo const & bi,
}
QStringList GuiCitation::citationStyles(BiblioInfo const & bi)
QStringList GuiCitation::citationStyles(BiblioInfo const & bi, size_t max_size)
{
docstring const before = qstring_to_ucs4(textBeforeED->text());
docstring const after = qstring_to_ucs4(textAfterED->text());
@ -598,7 +599,7 @@ QStringList GuiCitation::citationStyles(BiblioInfo const & bi)
vector<CitationStyle> styles = citeStyles_;
// FIXME: pass a dictionary instead of individual before, after, dialog, etc.
vector<docstring> ret = bi.getCiteStrings(keys, styles, documentBuffer(),
false, before, after, from_utf8("dialog"));
before, after, from_utf8("dialog"), max_size);
return to_qstring_list(ret);
}

View File

@ -123,7 +123,7 @@ private:
);
/// List of example cite strings
QStringList citationStyles(BiblioInfo const & bi);
QStringList citationStyles(BiblioInfo const & bi, size_t max_size);
/// Set the Params variable for the Controller.
void apply(int const choice, bool const full, bool const force,

View File

@ -1554,9 +1554,10 @@ void MenuDefinition::expandCiteStyles(BufferView const * bv)
vector<docstring> const keys = getVectorFromString(key);
vector<CitationStyle> const citeStyleList = buf->params().citeStyles();
static const size_t max_length = 40;
vector<docstring> citeStrings =
buf->masterBibInfo().getCiteStrings(keys, citeStyleList, bv->buffer(),
false, before, after, from_utf8("dialog"));
before, after, from_utf8("dialog"), max_length);
vector<docstring>::const_iterator cit = citeStrings.begin();
vector<docstring>::const_iterator end = citeStrings.end();

View File

@ -33,6 +33,7 @@
#include "support/lstrings.h"
#include <algorithm>
#include <climits>
using namespace std;
using namespace lyx::support;
@ -273,8 +274,8 @@ docstring InsetCitation::complexLabel(bool for_xhtml) const
buffer().params().documentClass().addCiteMacro("!textafter", to_utf8(after));
*/
docstring label;
vector<docstring> const keys = getVectorFromString(key);
label = biblist.getLabel(keys, buffer(), cite_type, for_xhtml, before, after);
vector<docstring> keys = getVectorFromString(key);
label = biblist.getLabel(keys, buffer(), cite_type, for_xhtml, UINT_MAX, before, after);
return label;
}