diff options
author | Marc Mutz <marc.mutz@kdab.com> | 2016-02-23 22:12:20 +0100 |
---|---|---|
committer | Marc Mutz <marc.mutz@kdab.com> | 2016-02-24 17:14:25 +0000 |
commit | aca859cbc4c599e0c996dd2f3df148d59e3f590a (patch) | |
tree | de541d53d1ce3a291d576adca9ff0361fae25a98 | |
parent | a4dee8e274f00a65bdd3ad706db6c39d4a82759d (diff) |
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>
-rw-r--r-- | src/corelib/mimetypes/qmimetypeparser.cpp | 26 |
1 files changed, 18 insertions, 8 deletions
diff --git a/src/corelib/mimetypes/qmimetypeparser.cpp b/src/corelib/mimetypes/qmimetypeparser.cpp index 0751c1feed..cdb50b25e7 100644 --- a/src/corelib/mimetypes/qmimetypeparser.cpp +++ b/src/corelib/mimetypes/qmimetypeparser.cpp @@ -172,13 +172,24 @@ bool QMimeTypeParserBase::parseNumber(const QStringRef &n, int *target, QString } #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 value = atts.value(QLatin1String(matchValueAttributeC)); const QStringRef offsets = atts.value(QLatin1String(matchOffsetAttributeC)); 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 @@ -266,19 +277,18 @@ bool QMimeTypeParserBase::parse(QIODevice *dev, const QString &fileName, QString } break; case ParseMagicMatchRule: { - QString magicErrorMessage; - QMimeMagicRule *rule = createMagicMatchRule(atts, &magicErrorMessage); - if (!rule->isValid()) - qWarning("QMimeDatabase: Error parsing %s\n%s", qPrintable(fileName), qPrintable(magicErrorMessage)); + auto result = createMagicMatchRule(atts); + if (Q_UNLIKELY(!result.rule.isValid())) + qWarning("QMimeDatabase: Error parsing %ls\n%ls", + qUtf16Printable(fileName), qUtf16Printable(result.errorMessage)); QList<QMimeMagicRule> *ruleList; if (currentRules.isEmpty()) ruleList = &rules; else // nest this rule into the proper parent ruleList = ¤tRules.top()->m_subMatches; - ruleList->append(*rule); + ruleList->append(std::move(result.rule)); //qDebug() << " MATCH added. Stack size was" << currentRules.size(); currentRules.push(&ruleList->last()); - delete rule; break; } case ParseError: |