From 03affacaa3d5530e96f869a8a6e900ca5e714918 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Sat, 5 Oct 2013 03:20:56 +0200
Subject: QCommandLineParser: pluck some low-hanging fruit re: exception safety

Make
  QCommandLineParser::add{Help,Version}Option()
  QCommandLineOption::setDefaultValue()
  QCommandLineOptionPrivate::setNames()
have transaction semantics: either they succeed, or they change nothing.
It's trivial to provide this guarantee, so do it.

Add a test for the surprising property that setDefaultValue("") resets
defaultValues() to an empty QStringList instead of one that contains
the empty string.

Change-Id: I61623019de3c7d2e52c24f42cc2e23ec5fddc4da
Reviewed-by: David Faure <david.faure@kdab.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/corelib/tools/qcommandlineoption.cpp                | 17 ++++++++++++-----
 src/corelib/tools/qcommandlineparser.cpp                |  4 ++--
 .../tools/qcommandlineparser/tst_qcommandlineparser.cpp | 12 ++++++++++++
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/src/corelib/tools/qcommandlineoption.cpp b/src/corelib/tools/qcommandlineoption.cpp
index 9827547c56..86f087674b 100644
--- a/src/corelib/tools/qcommandlineoption.cpp
+++ b/src/corelib/tools/qcommandlineoption.cpp
@@ -199,7 +199,8 @@ QStringList QCommandLineOption::names() const
 
 void QCommandLineOptionPrivate::setNames(const QStringList &nameList)
 {
-    names.clear();
+    QStringList newNames;
+    newNames.reserve(nameList.size());
     if (nameList.isEmpty())
         qWarning("QCommandLineOption: Options must have at least one name");
     foreach (const QString &name, nameList) {
@@ -214,9 +215,11 @@ void QCommandLineOptionPrivate::setNames(const QStringList &nameList)
             else if (name.contains(QLatin1Char('=')))
                 qWarning("QCommandLineOption: Option names cannot contain a '='");
             else
-                names.append(name);
+                newNames.append(name);
         }
     }
+    // commit
+    names.swap(newNames);
 }
 
 /*!
@@ -288,9 +291,13 @@ QString QCommandLineOption::description() const
  */
 void QCommandLineOption::setDefaultValue(const QString &defaultValue)
 {
-    d->defaultValues.clear();
-    if (!defaultValue.isEmpty())
-        d->defaultValues << defaultValue;
+    QStringList newDefaultValues;
+    if (!defaultValue.isEmpty()) {
+        newDefaultValues.reserve(1);
+        newDefaultValues << defaultValue;
+    }
+    // commit:
+    d->defaultValues.swap(newDefaultValues);
 }
 
 /*!
diff --git a/src/corelib/tools/qcommandlineparser.cpp b/src/corelib/tools/qcommandlineparser.cpp
index 5463e4f0c1..db4c2961f0 100644
--- a/src/corelib/tools/qcommandlineparser.cpp
+++ b/src/corelib/tools/qcommandlineparser.cpp
@@ -280,9 +280,9 @@ bool QCommandLineParser::addOption(const QCommandLineOption &option)
 */
 QCommandLineOption QCommandLineParser::addVersionOption()
 {
-    d->builtinVersionOption = true;
     QCommandLineOption opt(QStringList() << QStringLiteral("v") << QStringLiteral("version"), tr("Displays version information."));
     addOption(opt);
+    d->builtinVersionOption = true;
     return opt;
 }
 
@@ -300,7 +300,6 @@ QCommandLineOption QCommandLineParser::addVersionOption()
 */
 QCommandLineOption QCommandLineParser::addHelpOption()
 {
-    d->builtinHelpOption = true;
     QCommandLineOption opt(QStringList()
 #ifdef Q_OS_WIN
                 << QStringLiteral("?")
@@ -308,6 +307,7 @@ QCommandLineOption QCommandLineParser::addHelpOption()
                 << QStringLiteral("h")
                 << QStringLiteral("help"), tr("Displays this help."));
     addOption(opt);
+    d->builtinHelpOption = true;
     return opt;
 }
 
diff --git a/tests/auto/corelib/tools/qcommandlineparser/tst_qcommandlineparser.cpp b/tests/auto/corelib/tools/qcommandlineparser/tst_qcommandlineparser.cpp
index f37e192ad3..d111c53551 100644
--- a/tests/auto/corelib/tools/qcommandlineparser/tst_qcommandlineparser.cpp
+++ b/tests/auto/corelib/tools/qcommandlineparser/tst_qcommandlineparser.cpp
@@ -69,6 +69,7 @@ private slots:
     void testUnknownOptionErrorHandling();
     void testDoubleDash_data();
     void testDoubleDash();
+    void testDefaultValue();
     void testProcessNotCalled();
     void testEmptyArgsList();
     void testMissingOptionValue();
@@ -322,6 +323,17 @@ void tst_QCommandLineParser::testDoubleDash()
     QCOMPARE(parser.unknownOptionNames(), QStringList());
 }
 
+void tst_QCommandLineParser::testDefaultValue()
+{
+    QCommandLineOption opt(QStringLiteral("name"), QStringLiteral("desc"),
+                           QStringLiteral("valueName"), QStringLiteral("default"));
+    QCOMPARE(opt.defaultValues(), QStringList(QStringLiteral("default")));
+    opt.setDefaultValue(QStringLiteral(""));
+    QCOMPARE(opt.defaultValues(), QStringList());
+    opt.setDefaultValue(QStringLiteral("default"));
+    QCOMPARE(opt.defaultValues(), QStringList(QStringLiteral("default")));
+}
+
 void tst_QCommandLineParser::testProcessNotCalled()
 {
     QCoreApplication app(empty_argc, empty_argv);
-- 
cgit v1.2.3