Fix segfault in 'gmx -h'

Combination of two recent changes caused 'gmx -h' to segfault because of
an uninitialized variable.  This fix is in cmdlinehelpcontext.cpp.

Added a test for this case, which required some extra functionality in
the module manager.  This is all the other changes.

Change-Id: Ie845c287975575c51c42763c03ac99bca047699c

diff --git a/src/gromacs/commandline/cmdlinehelpcontext.cpp b/src/gromacs/commandline/cmdlinehelpcontext.cpp
index c7482a7c13b97d17a1b675067a464cc1d2ee9587..b2ae873e47ac0d5c8c61990a597c048e14219dd4 100644 (file)
--- a/src/gromacs/commandline/cmdlinehelpcontext.cpp
+++ b/src/gromacs/commandline/cmdlinehelpcontext.cpp
@@ -78,7 +78,8 @@ class CommandLineHelpContext::Impl
         }
         //! Creates an implementation class from a low-level context.
         explicit Impl(const HelpWriterContext &writerContext)
-            : writerContext_(writerContext), bHidden_(false)
+            : writerContext_(writerContext),
+              completionWriter_(NULL), bHidden_(false)
         {
         }
 

diff --git a/src/gromacs/commandline/cmdlinehelpmodule.cpp b/src/gromacs/commandline/cmdlinehelpmodule.cpp
index ea640e0277248e5a8b0495e45a0d5fd3e2afd165..2574edbd137025ee670464a349d6a7bb7b5bb995 100644 (file)
--- a/src/gromacs/commandline/cmdlinehelpmodule.cpp
+++ b/src/gromacs/commandline/cmdlinehelpmodule.cpp
@@ -98,6 +98,8 @@ class CommandLineHelpModuleImpl
         const CommandLineModuleInterface *moduleOverride_;
         bool                              bHidden_;
 
+        File                             *outputOverride_;
+
         GMX_DISALLOW_COPY_AND_ASSIGN(CommandLineHelpModuleImpl);
 };
 
@@ -723,7 +725,8 @@ CommandLineHelpModuleImpl::CommandLineHelpModuleImpl(
         const CommandLineModuleGroupList &groups)
     : rootTopic_(new RootHelpTopic), programContext_(programContext),
       binaryName_(binaryName), modules_(modules), groups_(groups),
-      context_(NULL), moduleOverride_(NULL), bHidden_(false)
+      context_(NULL), moduleOverride_(NULL), bHidden_(false),
+      outputOverride_(NULL)
 {
 }
 
@@ -801,6 +804,11 @@ void CommandLineHelpModule::setModuleOverride(
     impl_->moduleOverride_ = &module;
 }
 
+void CommandLineHelpModule::setOutputRedirect(File *output)
+{
+    impl_->outputOverride_ = output;
+}
+
 int CommandLineHelpModule::run(int argc, char *argv[])
 {
     // Add internal topics lazily here.
@@ -835,10 +843,15 @@ int CommandLineHelpModule::run(int argc, char *argv[])
         return 0;
     }
 
+    File *outputFile = &File::standardOutput();
+    if (impl_->outputOverride_ != NULL)
+    {
+        outputFile = impl_->outputOverride_;
+    }
     HelpLinks                                 links(eHelpOutputFormat_Console);
     initProgramLinks(&links, *impl_);
     boost::scoped_ptr<CommandLineHelpContext> context(
-            new CommandLineHelpContext(&File::standardOutput(),
+            new CommandLineHelpContext(outputFile,
                                        eHelpOutputFormat_Console, &links));
     context->setShowHidden(impl_->bHidden_);
     if (impl_->moduleOverride_ != NULL)

diff --git a/src/gromacs/commandline/cmdlinehelpmodule.h b/src/gromacs/commandline/cmdlinehelpmodule.h
index 19c5c0b6922b7126a87e414a9db6df9a96961253..a0eb12cc747a1bd68b3bc928b4c70ae36139d19f 100644 (file)
--- a/src/gromacs/commandline/cmdlinehelpmodule.h
+++ b/src/gromacs/commandline/cmdlinehelpmodule.h
@@ -52,6 +52,7 @@ namespace gmx
 {
 
 class CommandLineHelpContext;
+class File;
 class Options;
 class ProgramContextInterface;
 
@@ -116,6 +117,14 @@ class CommandLineHelpModule : public CommandLineModuleInterface
          */
         void setModuleOverride(const CommandLineModuleInterface &module);
 
+        /*! \brief
+         * Sets a file to write help output to instead of default `stdout`.
+         *
+         * Used for unit testing; see
+         * CommandLineModuleManager::setOutputRedirect() for more details.
+         */
+        void setOutputRedirect(File *output);
+
         virtual const char *name() const { return "help"; }
         virtual const char *shortDescription() const
         {

diff --git a/src/gromacs/commandline/cmdlinemodulemanager.cpp b/src/gromacs/commandline/cmdlinemodulemanager.cpp
index 7b510b4a01e998cb8b109f0b060b9fc7bae1f49a..c893bcb869fc92e3d60c970776ef12e422530bdc 100644 (file)
--- a/src/gromacs/commandline/cmdlinemodulemanager.cpp
+++ b/src/gromacs/commandline/cmdlinemodulemanager.cpp
@@ -516,6 +516,12 @@ void CommandLineModuleManager::setQuiet(bool bQuiet)
     impl_->bQuiet_ = bQuiet;
 }
 
+void CommandLineModuleManager::setOutputRedirect(File *output)
+{
+    impl_->ensureHelpModuleExists();
+    impl_->helpModule_->setOutputRedirect(output);
+}
+
 void CommandLineModuleManager::setSingleModule(CommandLineModuleInterface *module)
 {
     impl_->singleModule_ = module;

diff --git a/src/gromacs/commandline/cmdlinemodulemanager.h b/src/gromacs/commandline/cmdlinemodulemanager.h
index 900958df6759e201f522b0d109e85c0fb53e8411..9fb4432f826cca2518897862e31ab3d12a1456cd 100644 (file)
--- a/src/gromacs/commandline/cmdlinemodulemanager.h
+++ b/src/gromacs/commandline/cmdlinemodulemanager.h
@@ -54,6 +54,7 @@ class CommandLineModuleGroup;
 class CommandLineModuleGroupData;
 class CommandLineModuleInterface;
 class CommandLineProgramContext;
+class File;
 
 //! \addtogroup module_commandline
 //! \{
@@ -182,12 +183,26 @@ class CommandLineModuleManager
          *
          * \param[in] bQuiet  Whether the module manager should remain silent.
          *
-         * Normally, the module manager prints out some information to stderr
+         * Normally, the module manager prints out some information to `stderr`
          * before it starts the module and after it finishes.  This removes
          * that output, which is useful in particular for unit tests so that
-         * they don't spam stderr.
+         * they don't spam `stderr`.
          */
         void setQuiet(bool bQuiet);
+        /*! \brief
+         * Redirects the output of the module manager to a file.
+         *
+         * \param[in] output  File to write the output to.
+         *
+         * Normally, the module manager prints explicitly requested text such
+         * as help output to `stdout`, but this method can be used to redirect
+         * that output to a file.  This is used for unit tests, either to keep
+         * them quiet or to verify that output.  To keep implementation options
+         * open, behavior with `output == NULL` is undefined and should not be
+         * relied on.  For tests, there should only be need to call this a
+         * single time, right after creating the manager.
+         */
+        void setOutputRedirect(File *output);
 
         /*! \brief
          * Makes the manager always run a single module.

diff --git a/src/gromacs/commandline/tests/cmdlinemodulemanager.cpp b/src/gromacs/commandline/tests/cmdlinemodulemanager.cpp
index 3eedd5ec5da466870694671f98e91acc16f0bb3c..9be6c2886a026a70d89b4e96da0d585c3b60824d 100644 (file)
--- a/src/gromacs/commandline/tests/cmdlinemodulemanager.cpp
+++ b/src/gromacs/commandline/tests/cmdlinemodulemanager.cpp
@@ -52,10 +52,12 @@
 #include "gromacs/commandline/cmdlinemodule.h"
 #include "gromacs/commandline/cmdlinemodulemanager.h"
 #include "gromacs/commandline/cmdlineprogramcontext.h"
+#include "gromacs/utility/file.h"
 
 #include "gromacs/onlinehelp/tests/mock_helptopic.h"
 #include "testutils/cmdlinetest.h"
 #include "testutils/testasserts.h"
+#include "testutils/testfilemanager.h"
 
 namespace
 {
@@ -133,9 +135,13 @@ class CommandLineModuleManagerTest : public ::testing::Test
 
         gmx::CommandLineModuleManager &manager() { return *manager_; }
 
+        void ignoreManagerOutput();
+
     private:
         boost::scoped_ptr<gmx::CommandLineProgramContext> programContext_;
         boost::scoped_ptr<gmx::CommandLineModuleManager>  manager_;
+        gmx::test::TestFileManager                        fileManager_;
+        boost::scoped_ptr<gmx::File>                      outputFile_;
 };
 
 void CommandLineModuleManagerTest::initManager(
@@ -165,10 +171,32 @@ CommandLineModuleManagerTest::addHelpTopic(const char *name, const char *title)
     return *topic;
 }
 
+void CommandLineModuleManagerTest::ignoreManagerOutput()
+{
+    outputFile_.reset(
+            new gmx::File(fileManager_.getTemporaryFilePath("out.txt"), "w"));
+    manager().setOutputRedirect(outputFile_.get());
+}
+
 /********************************************************************
  * Actual tests
  */
 
+TEST_F(CommandLineModuleManagerTest, RunsGeneralHelp)
+{
+    const char *const cmdline[] = {
+        "test"
+    };
+    CommandLine       args(cmdline);
+    initManager(args, "test");
+    ignoreManagerOutput();
+    addModule("module", "First module");
+    addModule("other", "Second module");
+    int rc = 0;
+    ASSERT_NO_THROW_GMX(rc = manager().run(args.argc(), args.argv()));
+    ASSERT_EQ(0, rc);
+}
+
 TEST_F(CommandLineModuleManagerTest, RunsModule)
 {
     const char *const cmdline[] = {