From 47d94757075e338c480ba4d7d24948242a85a9bb Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Sat, 21 Jan 2012 11:45:15 -0500
Subject: [PATCH] Convert LLProcess consumers from LLSD to LLProcess::Params
 block. Using a Params block gives compile-time checking against attribute
 typos. One might inadvertently set myLLSD["autofill"] = false and only
 discover it when things behave strangely at runtime; but trying to set
 myParams.autofill will produce a compile error. However, it's excellent that
 the same LLProcess::create() method can accept either LLProcess::Params or a
 properly-constructed LLSD block.

---
 indra/llcommon/tests/llprocess_test.cpp       | 26 +++++++--------
 indra/llcommon/tests/llsdserialize_test.cpp   |  6 ++--
 indra/llplugin/llpluginprocessparent.cpp      | 28 ++++++++--------
 indra/llplugin/llpluginprocessparent.h        | 12 +++----
 indra/newview/llexternaleditor.cpp            | 33 ++++++++++---------
 indra/newview/llexternaleditor.h              |  5 ++-
 .../updater/llupdateinstaller.cpp             | 12 +++----
 7 files changed, 62 insertions(+), 60 deletions(-)

diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp
index 55e22abd819..405540e436c 100644
--- a/indra/llcommon/tests/llprocess_test.cpp
+++ b/indra/llcommon/tests/llprocess_test.cpp
@@ -107,8 +107,8 @@ struct PythonProcessLauncher
         const char* PYTHON(getenv("PYTHON"));
         tut::ensure("Set $PYTHON to the Python interpreter", PYTHON);
 
-        mParams["executable"] = PYTHON;
-        mParams["args"].append(mScript.getName());
+        mParams.executable = PYTHON;
+        mParams.args.add(mScript.getName());
     }
 
     /// Run Python script and wait for it to complete.
@@ -142,13 +142,13 @@ struct PythonProcessLauncher
     {
         NamedTempFile out("out", ""); // placeholder
         // pass name of this temporary file to the script
-        mParams["args"].append(out.getName());
+        mParams.args.add(out.getName());
         run();
         // assuming the script wrote to that file, read it
         return readfile(out.getName(), STRINGIZE("from " << mDesc << " script"));
     }
 
-    LLSD mParams;
+    LLProcess::Params mParams;
     LLProcessPtr mPy;
     std::string mDesc;
     NamedTempFile mScript;
@@ -515,7 +515,7 @@ namespace tut
                                  "with open(sys.argv[1], 'w') as f:\n"
                                  "    f.write(os.getcwd())\n");
         // Before running, call setWorkingDirectory()
-        py.mParams["cwd"] = tempdir.getName();
+        py.mParams.cwd = tempdir.getName();
         ensure_equals("os.getcwd()", py.run_read(), tempdir.getName());
     }
 
@@ -531,9 +531,9 @@ namespace tut
                                  "    for arg in sys.argv[1:]:\n"
                                  "        print >>f, arg\n");
         // We expect that PythonProcessLauncher has already appended
-        // its own NamedTempFile to mParams["args"] (sys.argv[0]).
-        py.mParams["args"].append("first arg");          // sys.argv[1]
-        py.mParams["args"].append("second arg");         // sys.argv[2]
+        // its own NamedTempFile to mParams.args (sys.argv[0]).
+        py.mParams.args.add("first arg");          // sys.argv[1]
+        py.mParams.args.add("second arg");         // sys.argv[2]
         // run_read() appends() one more argument, hence [3]
         std::string output(py.run_read());
         boost::split_iterator<std::string::const_iterator>
@@ -568,7 +568,7 @@ namespace tut
                                  "with open(sys.argv[1], 'w') as f:\n"
                                  "    f.write('bad')\n");
         NamedTempFile out("out", "not started");
-        py.mParams["args"].append(out.getName());
+        py.mParams.args.add(out.getName());
         py.mPy = LLProcess::create(py.mParams);
         ensure("couldn't launch kill() script", py.mPy);
         // Wait for the script to wake up and do its first write
@@ -611,7 +611,7 @@ namespace tut
                                      "# if caller hasn't managed to kill by now, bad\n"
                                      "with open(sys.argv[1], 'w') as f:\n"
                                      "    f.write('bad')\n");
-            py.mParams["args"].append(out.getName());
+            py.mParams.args.add(out.getName());
             py.mPy = LLProcess::create(py.mParams);
             ensure("couldn't launch kill() script", py.mPy);
             // Capture id for later
@@ -667,9 +667,9 @@ namespace tut
                                      "# okay, saw 'go', write 'ack'\n"
                                      "with open(sys.argv[1], 'w') as f:\n"
                                      "    f.write('ack')\n");
-            py.mParams["args"].append(from.getName());
-            py.mParams["args"].append(to.getName());
-            py.mParams["autokill"] = false;
+            py.mParams.args.add(from.getName());
+            py.mParams.args.add(to.getName());
+            py.mParams.autokill = false;
             py.mPy = LLProcess::create(py.mParams);
             ensure("couldn't launch kill() script", py.mPy);
             // Capture id for later
diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp
index 7756ba62260..e625545763a 100644
--- a/indra/llcommon/tests/llsdserialize_test.cpp
+++ b/indra/llcommon/tests/llsdserialize_test.cpp
@@ -1557,9 +1557,9 @@ namespace tut
             }
 
 #else  // LL_DARWIN, LL_LINUX
-            LLSD params;
-            params["executable"] = PYTHON;
-            params["args"].append(scriptfile.getName());
+            LLProcess::Params params;
+            params.executable = PYTHON;
+            params.args.add(scriptfile.getName());
             LLProcessPtr py(LLProcess::create(params));
             ensure(STRINGIZE("Couldn't launch " << desc << " script"), py);
             // Implementing timeout would mean messing with alarm() and
diff --git a/indra/llplugin/llpluginprocessparent.cpp b/indra/llplugin/llpluginprocessparent.cpp
index 9b225cabb8f..f10eaee5b48 100644
--- a/indra/llplugin/llpluginprocessparent.cpp
+++ b/indra/llplugin/llpluginprocessparent.cpp
@@ -163,8 +163,8 @@ void LLPluginProcessParent::errorState(void)
 
 void LLPluginProcessParent::init(const std::string &launcher_filename, const std::string &plugin_dir, const std::string &plugin_filename, bool debug)
 {	
-	mProcessParams["executable"] = launcher_filename;
-	mProcessParams["cwd"] = plugin_dir;
+	mProcessParams.executable = launcher_filename;
+	mProcessParams.cwd = plugin_dir;
 	mPluginFile = plugin_filename;
 	mPluginDir = plugin_dir;
 	mCPUUsage = 0.0f;
@@ -375,7 +375,7 @@ void LLPluginProcessParent::idle(void)
 				// Launch the plugin process.
 				
 				// Only argument to the launcher is the port number we're listening on
-				mProcessParams["args"].append(stringize(mBoundPort));
+				mProcessParams.args.add(stringize(mBoundPort));
 				if (! (mProcess = LLProcess::create(mProcessParams)))
 				{
 					errorState();
@@ -390,17 +390,17 @@ void LLPluginProcessParent::idle(void)
 						// The command we're constructing would look like this on the command line:
 						// osascript -e 'tell application "Terminal"' -e 'set win to do script "gdb -pid 12345"' -e 'do script "continue" in win' -e 'end tell'
 
-						LLSD params;
-						params["executable"] = "/usr/bin/osascript";
-						params["args"].append("-e");
-						params["args"].append("tell application \"Terminal\"");
-						params["args"].append("-e");
-						params["args"].append(STRINGIZE("set win to do script \"gdb -pid "
-														<< mProcess->getProcessID() << "\""));
-						params["args"].append("-e");
-						params["args"].append("do script \"continue\" in win");
-						params["args"].append("-e");
-						params["args"].append("end tell");
+						LLProcess::Params params;
+						params.executable = "/usr/bin/osascript";
+						params.args.add("-e");
+						params.args.add("tell application \"Terminal\"");
+						params.args.add("-e");
+						params.args.add(STRINGIZE("set win to do script \"gdb -pid "
+												  << mProcess->getProcessID() << "\""));
+						params.args.add("-e");
+						params.args.add("do script \"continue\" in win");
+						params.args.add("-e");
+						params.args.add("end tell");
 						mDebugger = LLProcess::create(params);
 
 						#endif
diff --git a/indra/llplugin/llpluginprocessparent.h b/indra/llplugin/llpluginprocessparent.h
index e8bcba75e07..990fc5cbae3 100644
--- a/indra/llplugin/llpluginprocessparent.h
+++ b/indra/llplugin/llpluginprocessparent.h
@@ -140,27 +140,27 @@ class LLPluginProcessParent : public LLPluginMessagePipeOwner
 	};
 	EState mState;
 	void setState(EState state);
-	
+
 	bool pluginLockedUp();
 	bool pluginLockedUpOrQuit();
 
 	bool accept();
-		
+
 	LLSocket::ptr_t mListenSocket;
 	LLSocket::ptr_t mSocket;
 	U32 mBoundPort;
 
-	LLSD mProcessParams;
+	LLProcess::Params mProcessParams;
 	LLProcessPtr mProcess;
-	
+
 	std::string mPluginFile;
 	std::string mPluginDir;
 
 	LLPluginProcessParentOwner *mOwner;
-	
+
 	typedef std::map<std::string, LLPluginSharedMemory*> sharedMemoryRegionsType;
 	sharedMemoryRegionsType mSharedMemoryRegions;
-	
+
 	LLSD mMessageClassVersions;
 	std::string mPluginVersionString;
 	
diff --git a/indra/newview/llexternaleditor.cpp b/indra/newview/llexternaleditor.cpp
index ba58cd80678..3dfebad958a 100644
--- a/indra/newview/llexternaleditor.cpp
+++ b/indra/newview/llexternaleditor.cpp
@@ -60,22 +60,22 @@ LLExternalEditor::EErrorCode LLExternalEditor::setCommand(const std::string& env
 	}
 
 	// Save command.
-	mProcessParams["executable"] = bin_path;
-	mProcessParams["args"].clear();
+	mProcessParams = LLProcess::Params();
+	mProcessParams.executable = bin_path;
 	for (size_t i = 1; i < tokens.size(); ++i)
 	{
-		mProcessParams["args"].append(tokens[i]);
+		mProcessParams.args.add(tokens[i]);
 	}
 
 	// Add the filename marker if missing.
 	if (cmd.find(sFilenameMarker) == std::string::npos)
 	{
-		mProcessParams["args"].append(sFilenameMarker);
+		mProcessParams.args.add(sFilenameMarker);
 		llinfos << "Adding the filename marker (" << sFilenameMarker << ")" << llendl;
 	}
 
 	llinfos << "Setting command [" << bin_path;
-	BOOST_FOREACH(const std::string& arg, llsd::inArray(mProcessParams["args"]))
+	BOOST_FOREACH(const std::string& arg, mProcessParams.args)
 	{
 		llcont << " \"" << arg << "\"";
 	}
@@ -86,33 +86,36 @@ LLExternalEditor::EErrorCode LLExternalEditor::setCommand(const std::string& env
 
 LLExternalEditor::EErrorCode LLExternalEditor::run(const std::string& file_path)
 {
-	if (mProcessParams["executable"].asString().empty() || ! mProcessParams["args"].size())
+	// LLInitParams type wrappers don't seem to have empty() or size()
+	// methods; try determining emptiness by comparing begin/end iterators.
+	if (std::string(mProcessParams.executable).empty() ||
+	    (mProcessParams.args.begin() == mProcessParams.args.end()))
 	{
 		llwarns << "Editor command not set" << llendl;
 		return EC_NOT_SPECIFIED;
 	}
 
 	// Copy params block so we can replace sFilenameMarker
-	LLSD params(mProcessParams);
+	LLProcess::Params params;
+	params.executable = mProcessParams.executable;
 
 	// Substitute the filename marker in the command with the actual passed file name.
-	LLSD& args(params["args"]);
-	for (LLSD::array_iterator ai(args.beginArray()), aend(args.endArray()); ai != aend; ++ai)
+	BOOST_FOREACH(const std::string& arg, mProcessParams.args)
 	{
-		std::string sarg(*ai);
-		LLStringUtil::replaceString(sarg, sFilenameMarker, file_path);
-		*ai = sarg;
+		std::string fixed(arg);
+		LLStringUtil::replaceString(fixed, sFilenameMarker, file_path);
+		params.args.add(fixed);
 	}
 
 	// Run the editor.
-	llinfos << "Running editor command [" << params["executable"];
-	BOOST_FOREACH(const std::string& arg, llsd::inArray(params["args"]))
+	llinfos << "Running editor command [" << std::string(params.executable);
+	BOOST_FOREACH(const std::string& arg, params.args)
 	{
 		llcont << " \"" << arg << "\"";
 	}
 	llcont << "]" << llendl;
 	// Prevent killing the process in destructor.
-	params["autokill"] = false;
+	params.autokill = false;
 	return LLProcess::create(params) ? EC_SUCCESS : EC_FAILED_TO_RUN;
 }
 
diff --git a/indra/newview/llexternaleditor.h b/indra/newview/llexternaleditor.h
index e81c360c24b..fd2c25020cb 100644
--- a/indra/newview/llexternaleditor.h
+++ b/indra/newview/llexternaleditor.h
@@ -27,7 +27,7 @@
 #ifndef LL_LLEXTERNALEDITOR_H
 #define LL_LLEXTERNALEDITOR_H
 
-#include "llsd.h"
+#include "llprocess.h"
 
 /**
  * Usage:
@@ -97,8 +97,7 @@ class LLExternalEditor
 	 */
 	static const std::string sSetting;
 
-
-	LLSD				mProcessParams;
+	LLProcess::Params		mProcessParams;
 };
 
 #endif // LL_LLEXTERNALEDITOR_H
diff --git a/indra/viewer_components/updater/llupdateinstaller.cpp b/indra/viewer_components/updater/llupdateinstaller.cpp
index e99fd0af7ee..2f87d59373a 100644
--- a/indra/viewer_components/updater/llupdateinstaller.cpp
+++ b/indra/viewer_components/updater/llupdateinstaller.cpp
@@ -78,12 +78,12 @@ int ll_install_update(std::string const & script,
 	llinfos << "UpdateInstaller: installing " << updatePath << " using " <<
 		actualScriptPath << LL_ENDL;
 	
-	LLSD params;
-	params["executable"] = actualScriptPath;
-	params["args"].append(updatePath);
-	params["args"].append(ll_install_failed_marker_path());
-	params["args"].append(boost::lexical_cast<std::string>(required));
-	params["autokill"] = false;
+	LLProcess::Params params;
+	params.executable = actualScriptPath;
+	params.args.add(updatePath);
+	params.args.add(ll_install_failed_marker_path());
+	params.args.add(boost::lexical_cast<std::string>(required));
+	params.autokill = false;
 	return LLProcess::create(params)? 0 : -1;
 }
 
-- 
GitLab