Build Timidity++ args separately on non-Windows

Rather than building a command line that's going to be manually split into
individual arguments passed to execvp, build the individual arguments directly.

src/sound/mididevices/music_timiditypp_mididevice.cpp

@@ -34,6 +34,8 @@
 
 #include "i_midi_win32.h"
 
+#include <string>
+#include <vector>
 
 #include "i_musicinterns.h"
 #include "c_cvars.h"
@@ -46,6 +48,7 @@
 
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <wordexp.h>
 #include <glob.h>
 #include <signal.h>
 
@@ -81,14 +84,16 @@ protected:
 	HANDLE ReadWavePipe;
 	HANDLE WriteWavePipe;
 	HANDLE ChildProcess;
+	FString CommandLine;
+	size_t LoopPos;
 	bool Validated;
 	bool ValidateTimidity();
 #else // _WIN32
 	int WavePipe[2];
 	pid_t ChildProcess;
 #endif
-	FString CommandLine;
+	FString ExeName;
-	size_t LoopPos;
+	bool Looping;
 
 	static bool FillStream(SoundStream *stream, void *buff, int len, void *userdata);
 #ifdef _WIN32
@@ -162,20 +167,24 @@ TimidityPPMIDIDevice::TimidityPPMIDIDevice(const char *args)
 #ifdef _WIN32
 	  ReadWavePipe(INVALID_HANDLE_VALUE), WriteWavePipe(INVALID_HANDLE_VALUE),
 	  ChildProcess(INVALID_HANDLE_VALUE),
-	  Validated(false)
+	  Validated(false),
 #else
-	  ChildProcess(-1)
+	  ChildProcess(-1),
 #endif
+	  Looping(false)
 {
 #ifndef _WIN32
 	WavePipe[0] = WavePipe[1] = -1;
 #endif
 
 	if (args == NULL || *args == 0) args = timidity_exe;
+	ExeName = args;
 
+#ifdef _WIN32
 	CommandLine.Format("%s %s -EFchorus=%s -EFreverb=%s -s%d ",
 		args, *timidity_extargs,
 		*timidity_chorus, *timidity_reverb, *timidity_frequency);
+#endif
 
 	if (DiskName == NULL)
 	{
@@ -229,14 +238,17 @@ bool TimidityPPMIDIDevice::Preprocess(MIDIStreamer *song, bool looping)
 	bool success;
 	FILE *f;
 
-	if (CommandLine.IsEmpty())
+	if (ExeName.IsEmpty())
 	{
 		return false;
 	}
 
 	// Tell TiMidity++ whether it should loop or not
+#ifdef _WIN32
 	CommandLine.LockBuffer()[LoopPos] = looping ? 'l' : ' ';
 	CommandLine.UnlockBuffer();
+#endif
+	Looping = looping;
 
 	// Write MIDI song to temporary file
 	song->CreateSMF(midi, looping ? 0 : 1);
@@ -327,12 +339,15 @@ int TimidityPPMIDIDevice::Open(MidiCallback callback, void *userdata)
 			Printf(PRINT_BOLD, "If your soundcard cannot play more than one\n"
 								"wave at a time, you will hear no music.\n");
 		}
+#ifdef _WIN32
 		else
 		{
 			CommandLine += "-o - -Ors";
 		}
+#endif
 	}
 
+#ifdef _WIN32
 	if (pipeSize == 0)
 	{
 		CommandLine += "-Od";
@@ -349,6 +364,7 @@ int TimidityPPMIDIDevice::Open(MidiCallback callback, void *userdata)
 
 	CommandLine += " -idl ";
 	CommandLine += DiskName.GetName();
+#endif
 	return 0;
 }
 
@@ -456,6 +472,7 @@ bool TimidityPPMIDIDevice::ValidateTimidity()
 
 bool TimidityPPMIDIDevice::LaunchTimidity ()
 {
+#ifdef _WIN32
 	if (CommandLine.IsEmpty())
 	{
 		return false;
@@ -463,7 +480,6 @@ bool TimidityPPMIDIDevice::LaunchTimidity ()
 
 	DPrintf (DMSG_NOTIFY, "cmd: \x1cG%s\n", CommandLine.GetChars());
 
-#ifdef _WIN32
 	STARTUPINFO startup = { sizeof(startup), };
 	PROCESS_INFORMATION procInfo;
 
@@ -509,6 +525,11 @@ bool TimidityPPMIDIDevice::LaunchTimidity ()
 	}
 	return false;
 #else
+	if (ExeName.IsEmpty())
+	{
+		return false;
+	}
+
 	if (WavePipe[0] != -1 && WavePipe[1] == -1 && Stream != NULL)
 	{
 		// Timidity was previously launched, so the write end of the pipe
@@ -523,78 +544,50 @@ bool TimidityPPMIDIDevice::LaunchTimidity ()
 	}
 
 	int forkres;
+	wordexp_t words;
 	glob_t glb;
 
 	// Get timidity executable path
-	int spaceIdx = 0;
+	const char *exename = "timidity"; // Fallback default
-	int spaceInExePathCount = -1;
+	glob(ExeName.GetChars(), 0, NULL, &glb);
-	FString TimidityExe;
+	if(glb.gl_pathc != 0)
-	do
+		exename = glb.gl_pathv[0];
-	{
+	// Get user-defined extra args
-		spaceIdx = CommandLine.IndexOf(' ', spaceIdx);
+	wordexp(timidity_extargs, &words, WRDE_NOCMD);
-		TimidityExe = CommandLine.Left(spaceIdx);
-		glob(TimidityExe.GetChars(), 0, NULL, &glb);
-		spaceIdx += 1;
-		spaceInExePathCount += 1;
-	} while (spaceIdx != 0 && glb.gl_pathc == 0);
-	if (spaceIdx == 0)
-	{
-		TimidityExe = FString("timidity"); // Maybe it's in your PATH?
-		spaceInExePathCount = 0;
-	}
-	globfree(&glb);
 
-	int strCount = 1;
+	std::string chorusarg = std::string("-EFchorus=") + *timidity_chorus;
-	for (spaceIdx = 0; spaceIdx < static_cast<int>(CommandLine.Len()); spaceIdx++)
+	std::string reverbarg = std::string("-EFreverb=") + *timidity_reverb;
-	{
+	std::string sratearg = std::string("-s") + std::to_string(*timidity_frequency);
-		if (CommandLine[spaceIdx] == ' ')
+	std::string outfilearg = "-o"; // An extra "-" is added later
-		{
+	std::string outmodearg = "-Or";
-			++strCount;
+	outmodearg += timidity_8bit ? "u8" : "s1";
-			if (CommandLine[spaceIdx+1] == ' ')
+	outmodearg += timidity_stereo ? "S" : "M";
-			{
+	if(timidity_byteswap) outmodearg += "x";
-				--strCount;
+	std::string ifacearg = "-id";
-			}
+	if(Looping) ifacearg += "l";
-		}
-	}
-	strCount -= spaceInExePathCount;
 
-	char** TimidityArgs = new char*[strCount + 1];
+	std::vector<const char*> arglist;
-	TimidityArgs[strCount] = NULL;
+	arglist.push_back(exename);
+	for(size_t i = 0;i < words.we_wordc;i++)
+		arglist.push_back(words.we_wordv[i]);
+	arglist.push_back(chorusarg.c_str());
+	arglist.push_back(reverbarg.c_str());
+	arglist.push_back(sratearg.c_str());
+	arglist.push_back(outfilearg.c_str());
+	arglist.push_back("-");
+	arglist.push_back(outmodearg.c_str());
+	arglist.push_back(ifacearg.c_str());
+	arglist.push_back(DiskName.GetName());
 
-	spaceIdx = CommandLine.IndexOf(' ');
+	DPrintf(DMSG_NOTIFY, "Timidity EXE: \x1cG%s\n", exename);
-	int curSpace = spaceIdx, i = 1;
+	int i = 1;
-
+	std::for_each(arglist.begin()+1, arglist.end(),
-	TimidityArgs[0] = new char[TimidityExe.Len() + 1];
+		[&i](const char *arg)
-	TimidityArgs[0][TimidityExe.Len()] = 0;
+		{ DPrintf(DMSG_NOTIFY, "arg %d: \x1cG%s\n", i++, arg); }
-	strcpy(TimidityArgs[0], TimidityExe.GetChars());
+	);
-
+	arglist.push_back(nullptr);
-	int argLen;
-	while (curSpace != -1)
-	{
-		curSpace = CommandLine.IndexOf(' ', spaceIdx);
-		if (curSpace != spaceIdx)
-		{
-			argLen = curSpace - spaceIdx + 1;
-			if (argLen < 0)
-			{
-				argLen = CommandLine.Len() - curSpace;
-			}
-			TimidityArgs[i] = new char[argLen];
-			TimidityArgs[i][argLen-1] = 0;
-			strcpy(TimidityArgs[i], CommandLine.Mid(spaceIdx, curSpace - spaceIdx).GetChars());
-			i += 1;
-		}
-		spaceIdx = curSpace + 1;
-	}
-
-	DPrintf(DMSG_NOTIFY, "Timidity EXE: \x1cG%s\n", TimidityExe.GetChars());
-	for (i = 0; i < strCount; i++)
-	{
-		DPrintf(DMSG_NOTIFY, "arg %d: \x1cG%s\n", i, TimidityArgs[i]);
-	}
 
 	forkres = fork ();
-
 	if (forkres == 0)
 	{
 		close (WavePipe[0]);
@@ -603,7 +596,7 @@ bool TimidityPPMIDIDevice::LaunchTimidity ()
 //		freopen ("/dev/null", "w", stderr);
 		close (WavePipe[1]);
 
-		execvp (TimidityExe.GetChars(), TimidityArgs);
+		execvp (exename, const_cast<char*const*>(arglist.data()));
 		fprintf(stderr,"execvp failed: %s\n", strerror(errno));
 		_exit (0);	// if execvp succeeds, we never get here
 	}
@@ -624,12 +617,7 @@ bool TimidityPPMIDIDevice::LaunchTimidity ()
 		}*/
 	}
 
-	for (i = 0; i < strCount; i++)
+	wordfree(&words);
-	{
-		delete [] TimidityArgs[i];
-	}
-
-	delete [] TimidityArgs;
 	globfree (&glb);
 	return ChildProcess != -1;
 #endif // _WIN32