QMimeTypeParser: don't use the heap to create QMimeMagicRules

The function createMagicMatchRule() returned a heap-allocated
QMimeMagicRule, and the caller code did not check the return
value for nullptr, but copied the rule into a container before
deleting the original again.

Fix by returning by-value instead. Every C++ compiler will
use RVO for this. On top, add an optimistic std::move()
when inserting the rule into the container (currently QList,
so no rvalue-push_back, yet).

While touching the return value, also remove an unholy
out-parameter with just local effects by returning a Result
struct instead. The rest of the code remains full of out-
parameters, of course.

Add one Q_UNLIKELY and two qUtf16Printable() as drive-bys.

Saves ~300b in text size on optimized GCC 5.3 Linux AMD64
builds.

Change-Id: I4374ab41f38502cd5c64ac37d106ca4bc6e00327
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
This commit is contained in:
Marc Mutz 2016-02-23 22:12:20 +01:00
parent a4dee8e274
commit aca859cbc4

View File

@ -172,13 +172,24 @@ bool QMimeTypeParserBase::parseNumber(const QStringRef &n, int *target, QString
} }
#ifndef QT_NO_XMLSTREAMREADER #ifndef QT_NO_XMLSTREAMREADER
static QMimeMagicRule *createMagicMatchRule(const QXmlStreamAttributes &atts, QString *errorMessage) struct CreateMagicMatchRuleResult {
QString errorMessage; // must be first
QMimeMagicRule rule;
CreateMagicMatchRuleResult(const QStringRef &type, const QStringRef &value, const QStringRef &offsets, const QStringRef &mask)
: errorMessage(), rule(type.toString(), value.toUtf8(), offsets.toString(), mask.toLatin1(), &errorMessage)
{
}
};
static CreateMagicMatchRuleResult createMagicMatchRule(const QXmlStreamAttributes &atts)
{ {
const QStringRef type = atts.value(QLatin1String(matchTypeAttributeC)); const QStringRef type = atts.value(QLatin1String(matchTypeAttributeC));
const QStringRef value = atts.value(QLatin1String(matchValueAttributeC)); const QStringRef value = atts.value(QLatin1String(matchValueAttributeC));
const QStringRef offsets = atts.value(QLatin1String(matchOffsetAttributeC)); const QStringRef offsets = atts.value(QLatin1String(matchOffsetAttributeC));
const QStringRef mask = atts.value(QLatin1String(matchMaskAttributeC)); const QStringRef mask = atts.value(QLatin1String(matchMaskAttributeC));
return new QMimeMagicRule(type.toString(), value.toUtf8(), offsets.toString(), mask.toLatin1(), errorMessage); return CreateMagicMatchRuleResult(type, value, offsets, mask);
} }
#endif #endif
@ -266,19 +277,18 @@ bool QMimeTypeParserBase::parse(QIODevice *dev, const QString &fileName, QString
} }
break; break;
case ParseMagicMatchRule: { case ParseMagicMatchRule: {
QString magicErrorMessage; auto result = createMagicMatchRule(atts);
QMimeMagicRule *rule = createMagicMatchRule(atts, &magicErrorMessage); if (Q_UNLIKELY(!result.rule.isValid()))
if (!rule->isValid()) qWarning("QMimeDatabase: Error parsing %ls\n%ls",
qWarning("QMimeDatabase: Error parsing %s\n%s", qPrintable(fileName), qPrintable(magicErrorMessage)); qUtf16Printable(fileName), qUtf16Printable(result.errorMessage));
QList<QMimeMagicRule> *ruleList; QList<QMimeMagicRule> *ruleList;
if (currentRules.isEmpty()) if (currentRules.isEmpty())
ruleList = &rules; ruleList = &rules;
else // nest this rule into the proper parent else // nest this rule into the proper parent
ruleList = &currentRules.top()->m_subMatches; ruleList = &currentRules.top()->m_subMatches;
ruleList->append(*rule); ruleList->append(std::move(result.rule));
//qDebug() << " MATCH added. Stack size was" << currentRules.size(); //qDebug() << " MATCH added. Stack size was" << currentRules.size();
currentRules.push(&ruleList->last()); currentRules.push(&ruleList->last());
delete rule;
break; break;
} }
case ParseError: case ParseError: