Refactoring and cleanup. The progress report mechanism should now be much

more flexible. May need this in the alphabranch...


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@32903 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/private/shared/CommandPipe.h

@@ -22,63 +22,72 @@ class BString;
 namespace BPrivate {
 
 class BCommandPipe {
-	public:
+public:
 								BCommandPipe();
 	virtual						~BCommandPipe();
-		
+
 			status_t			AddArg(const char* argv);
 			void				PrintToStream() const;
-		
+
 	// FlushArgs() deletes the commands while Close() explicity closes all
 	// pending pipe-ends
 	// Note: Internally FlushArgs() calls Close()
 			void				FlushArgs();
 			void				Close();
-		
+
+	// You MUST NOT free/delete the strings in the array, but you MUST free
+	// just the array itself.
+			const char**		Argv(int32& _argc) const;
+
 	// If you use these, you must explicitly call "close" for the parameters
-	// (outdes/errdes) when you are done with them!
+	// (stdOut/stdErr) when you are done with them!
-			thread_id			Pipe(int* outdes, int* errdes) const;
+			thread_id			Pipe(int* stdOut, int* stdErr) const;
-			thread_id			Pipe(int* outdes) const;
+			thread_id			Pipe(int* stdOut) const;
-			thread_id			PipeAll(int* outAndErrDes) const;
+			thread_id			PipeAll(int* stdOutAndErr) const;
-		
+
 	// If you use these, you need NOT call "fclose" for the parameters
 	// (out/err) when you are done with them, also you need not do any
 	// allocation for these FILE* pointers, just use FILE* out = NULL
 	// and pass &out and so on...
 			thread_id			PipeInto(FILE** _out, FILE** _err);
 			thread_id			PipeInto(FILE** _outAndErr);
-		
+
 	// Run() is a synchronous call, and waits till the command has finished
 	// executing RunAsync() is an asynchronous call that returns immediately
 	// after launching the command Neither of these bother about redirecting
 	// pipes for you to use
 			void				Run();
 			void				RunAsync();
-		
+
-	// This function reads line-by-line from "file", cancels its reading
+	// Reading the Pipe output
-	// when "*cancel" is true. It reports each line it has read to "target"
+
-	// using the supplied "_message" and string field name. "cancel" can be
+			class LineReader {
-	// NULL
+			public:
-			BString				ReadLines(FILE* file, bool* cancel,
+				virtual	bool		IsCanceled() = 0;
-									BMessenger& target, const BMessage& message,
+				virtual	status_t	ReadLine(const BString& line) = 0;
-									const BString& stringFieldName);
+				// TODO: Add a Timeout() method.
-		
+			};
-	// You need NOT free/delete	the return array of strings
+
-			const char**		Argv(int32& _argc) const;
+	// This function reads line-by-line from "file". It calls IsCanceled()
-		
+	// on the passed LineReader instance for each byte read from file
+	// (currently). And it calls ReadLine() for each complete line.
+			status_t			ReadLines(FILE* file, LineReader* lineReader);
+	// This method can be used to read the entire file into a BString.
+			BString				ReadLines(FILE* file);
+
 	// Stardard append operators, if you use pointers to a BCommandPipe,
 	// you must use *pipe << "command"; and not pipe << "command" (as it
 	// will not compile that way)
-			BCommandPipe&		operator<<(const char *arg);
+			BCommandPipe&		operator<<(const char* arg);
 			BCommandPipe&		operator<<(const BString& arg);
 			BCommandPipe&		operator<<(const BCommandPipe& arg);
-		
+
-	protected:
+protected:
 			BList				fArgList;
-			int					fOutDes[2];
+			int					fStdOut[2];
-			int					fErrDes[2];
+			int					fStdErr[2];
-			bool				fOutDesOpen;
+			bool				fStdOutOpen;
-			bool				fErrDesOpen;
+			bool				fStdErrOpen;
 };
 
 }	// namespace BPrivate

src/kits/shared/CommandPipe.cpp

@@ -4,6 +4,7 @@
  *
  * Authors:
  *		Ramshankar, v.ramshankar@gmail.com
+ *		Stephan Aßmus <superstippi@gmx.de>
  */
 
 //! BCommandPipe class to handle reading shell output
@@ -20,8 +21,9 @@
 
 
 BCommandPipe::BCommandPipe()
-	: fOutDesOpen(false)
+	:
-	, fErrDesOpen(false)
+	fStdOutOpen(false),
+	fStdErrOpen(false)
 {
 }
 
@@ -35,8 +37,19 @@ BCommandPipe::~BCommandPipe()
 status_t
 BCommandPipe::AddArg(const char* arg)
 {
-	return (fArgList.AddItem(reinterpret_cast<void*>(strdup(arg))) == true ?
+	if (arg == NULL || arg[0] == '\0')
-		B_OK : B_ERROR);
+		return B_BAD_VALUE;
+
+	char* argCopy = strdup(arg);
+	if (argCopy == NULL)
+		return B_NO_MEMORY;
+
+	if (!fArgList.AddItem(reinterpret_cast<void*>(argCopy))) {
+		free(argCopy);
+		return B_NO_MEMORY;
+	}
+
+	return B_OK;
 }
 
 
@@ -44,7 +57,7 @@ void
 BCommandPipe::PrintToStream() const
 {
 	for (int32 i = 0L; i < fArgList.CountItems(); i++)
-		printf("%s ", (char*)fArgList.ItemAtFast(i));
+		printf("%s ", reinterpret_cast<char*>(fArgList.ItemAtFast(i)));
 
 	printf("\n");
 }
@@ -54,10 +67,10 @@ void
 BCommandPipe::FlushArgs()
 {
 	// Delete all arguments from the list
-	for (int32 i = 0; i < fArgList.CountItems(); i++)
+	for (int32 i = fArgList.CountItems() - 1; i >= 0; i--)
-		free(fArgList.RemoveItem(0L));
+		free(fArgList.ItemAtFast(i));
-
 	fArgList.MakeEmpty();
+
 	Close();
 }
 
@@ -65,29 +78,29 @@ BCommandPipe::FlushArgs()
 void
 BCommandPipe::Close()
 {
-	if (fErrDesOpen) {
+	if (fStdErrOpen) {
-		close(fErrDes[0]);
+		close(fStdErr[0]);
-		fErrDesOpen = false;
+		fStdErrOpen = false;
 	}
-	
+
-	if (fOutDesOpen) {
+	if (fStdOutOpen) {
-		close(fOutDes[0]);
+		close(fStdOut[0]);
-		fOutDesOpen = false;
+		fStdOutOpen = false;
 	}
 }
 
 
 const char**
-BCommandPipe::Argv(int32& _argc) const
+BCommandPipe::Argv(int32& argc) const
 {
-	// *** Warning *** Freeing is left to caller!! Indicated in Header
+	// NOTE: Freeing is left to caller as indicated in the header!
-	int32 argc = fArgList.CountItems();
+	argc = fArgList.CountItems();
-	const char **argv = (const char**)malloc((argc + 1) * sizeof(char*));
+	const char** argv = reinterpret_cast<const char**>(
+		malloc((argc + 1) * sizeof(char*)));
 	for (int32 i = 0; i < argc; i++)
-		argv[i] = (const char*)fArgList.ItemAtFast(i);
+		argv[i] = reinterpret_cast<const char*>(fArgList.ItemAtFast(i));
-	
+
 	argv[argc] = NULL;
-	_argc = argc;
 	return argv;
 }
 
@@ -96,88 +109,81 @@ BCommandPipe::Argv(int32& _argc) const
 
 
 thread_id
-BCommandPipe::PipeAll(int* outAndErrDes) const
+BCommandPipe::PipeAll(int* stdOutAndErr) const
 {
 	// This function pipes both stdout and stderr to the same filedescriptor
-	// (outdes)
+	// (stdOut)
-	int oldstdout;
+	int oldStdOut;
-	int oldstderr;
+	int oldStdErr;
-	pipe(outAndErrDes);
+	pipe(stdOutAndErr);
-	oldstdout = dup(STDOUT_FILENO);
+	oldStdOut = dup(STDOUT_FILENO);
-	oldstderr = dup(STDERR_FILENO);
+	oldStdErr = dup(STDERR_FILENO);
 	close(STDOUT_FILENO);
 	close(STDERR_FILENO);
-	dup2(outAndErrDes[1], STDOUT_FILENO);
+	// TODO: This looks broken, using "stdOutAndErr[1]" twice!
-	dup2(outAndErrDes[1], STDERR_FILENO);
+	dup2(stdOutAndErr[1], STDOUT_FILENO);
-	
+	dup2(stdOutAndErr[1], STDERR_FILENO);
+
 	// Construct the argv vector
-	int32 argc = fArgList.CountItems();
+	int32 argc;
-	const char **argv = (const char**)malloc((argc + 1) * sizeof(char*));
+	const char** argv = Argv(argc);
-	for (int32 i = 0; i < argc; i++)
+
-		argv[i] = (const char*)fArgList.ItemAtFast(i);
-	
-	argv[argc] = NULL;
-	
 	// Load the app image... and pass the args
-	thread_id appThread = load_image((int)argc, argv, const_cast<
+	thread_id appThread = load_image((int)argc, argv,
-		const char**>(environ));
+		const_cast<const char**>(environ));
 
-	dup2(oldstdout, STDOUT_FILENO);
+	dup2(oldStdOut, STDOUT_FILENO);
-	dup2(oldstderr, STDERR_FILENO);
+	dup2(oldStdErr, STDERR_FILENO);
-	close(oldstdout);
+	close(oldStdOut);
-	close(oldstderr);
+	close(oldStdErr);
+
+	free(argv);
 
-	delete[] argv;
-	
 	return appThread;
 }
 
 
 thread_id
-BCommandPipe::Pipe(int* outdes, int* errdes) const
+BCommandPipe::Pipe(int* stdOut, int* stdErr) const
 {
-	int oldstdout;
+	int oldStdOut;
-	int oldstderr;
+	int oldStdErr;
-	pipe(outdes);
+	pipe(stdOut);
-	pipe(errdes);
+	pipe(stdErr);
-	oldstdout = dup(STDOUT_FILENO);
+	oldStdOut = dup(STDOUT_FILENO);
-	oldstderr = dup(STDERR_FILENO);
+	oldStdErr = dup(STDERR_FILENO);
 	close(STDOUT_FILENO);
 	close(STDERR_FILENO);
-	dup2(outdes[1], STDOUT_FILENO);
+	dup2(stdOut[1], STDOUT_FILENO);
-	dup2(errdes[1], STDERR_FILENO);
+	dup2(stdErr[1], STDERR_FILENO);
-	
+
 	// Construct the argv vector
-	int32 argc = fArgList.CountItems();
+	int32 argc;
-	const char **argv = (const char**)malloc((argc + 1) * sizeof(char*));
+	const char** argv = Argv(argc);
-	for (int32 i = 0; i < argc; i++)
+
-		argv[i] = (const char*)fArgList.ItemAtFast(i);
-	
-	argv[argc] = NULL;
-	
 	// Load the app image... and pass the args
 	thread_id appThread = load_image((int)argc, argv, const_cast<
 		const char**>(environ));
 
-	dup2(oldstdout, STDOUT_FILENO);
+	dup2(oldStdOut, STDOUT_FILENO);
-	dup2(oldstderr, STDERR_FILENO);
+	dup2(oldStdErr, STDERR_FILENO);
-	close(oldstdout);
+	close(oldStdOut);
-	close(oldstderr);
+	close(oldStdErr);
-	
+
-	delete[] argv;
+	free(argv);
 
 	return appThread;
 }
 
 
 thread_id
-BCommandPipe::Pipe(int* outdes) const
+BCommandPipe::Pipe(int* stdOut) const
 {
 	// Redirects only output (stdout) to caller, stderr is closed
-	int errdes[2];
+	int stdErr[2];
-	thread_id tid = Pipe(outdes, errdes);
+	thread_id tid = Pipe(stdOut, stdErr);
-	close(errdes[0]);
+	close(stdErr[0]);
-	close(errdes[1]);
+	close(stdErr[1]);
 	return tid;
 }
 
@@ -186,19 +192,20 @@ thread_id
 BCommandPipe::PipeInto(FILE** _out, FILE** _err)
 {
 	Close();
-	thread_id tid = Pipe(fOutDes, fErrDes);
-	
-	resume_thread(tid);
-	
-	close(fErrDes[1]);
-	close(fOutDes[1]);
-	
-	fOutDesOpen = true;
-	fErrDesOpen = true;
 
-	*_out = fdopen(fOutDes[0], "r");
+	thread_id tid = Pipe(fStdOut, fStdErr);
-	*_err = fdopen(fErrDes[0], "r");
+	if (tid >= 0)
-	
+		resume_thread(tid);
+
+	close(fStdErr[1]);
+	close(fStdOut[1]);
+
+	fStdOutOpen = true;
+	fStdErrOpen = true;
+
+	*_out = fdopen(fStdOut[0], "r");
+	*_err = fdopen(fStdErr[0], "r");
+
 	return tid;
 }
 
@@ -207,17 +214,15 @@ thread_id
 BCommandPipe::PipeInto(FILE** _outAndErr)
 {
 	Close();
-	thread_id tid = PipeAll(fOutDes);
+
-	
+	thread_id tid = PipeAll(fStdOut);
-	if (tid == B_ERROR || tid == B_NO_MEMORY)
+	if (tid >= 0)
-		return tid;
+		resume_thread(tid);
-	
+
-	resume_thread(tid);
+	close(fStdOut[1]);
-	
+	fStdOutOpen = true;
-	close(fOutDes[1]);
+
-	fOutDesOpen = true;
+	*_outAndErr = fdopen(fStdOut[0], "r");
-	
-	*_outAndErr = fdopen(fOutDes[0], "r");
 	return tid;
 }
 
@@ -230,14 +235,14 @@ BCommandPipe::Run()
 {
 	// Runs the command without bothering to redirect streams, this is similar
 	// to system() but uses pipes and wait_for_thread.... Synchronous.
-	int outdes[2], errdes[2];
+	int stdOut[2], stdErr[2];
 	status_t exitCode;
-	wait_for_thread(Pipe(outdes, errdes), &exitCode);
+	wait_for_thread(Pipe(stdOut, stdErr), &exitCode);
 
-	close(outdes[0]);
+	close(stdOut[0]);
-	close(errdes[0]);
+	close(stdErr[0]);
-	close(outdes[1]);
+	close(stdOut[1]);
-	close(errdes[1]);
+	close(stdErr[1]);
 }
 
 
@@ -256,69 +261,107 @@ BCommandPipe::RunAsync()
 // #pragma mark -
 
 
-BString
+status_t
-BCommandPipe::ReadLines(FILE* file, bool* cancel, BMessenger& target,
+BCommandPipe::ReadLines(FILE* file, LineReader* lineReader)
-	const BMessage& message, const BString& stringFieldName)
 {
-	// Reads output of file, line by line. The entire output is returned
+	// Reads output of file, line by line. Each line is passed to lineReader
-	// and as each line is being read "target" (if any) is informed,
+	// for inspection, and the IsCanceled() method is repeatedly called.
-	// with "message" i.e. AddString (stringFieldName, <line read from pipe>)
+
-	
+	if (file == NULL || lineReader == NULL)
-	// "cancel" cancels the reading process, when it becomes true (unless its
+		return B_BAD_VALUE;
-	// waiting on fgetc()) and I don't know how to cancel the waiting fgetc()
+
-	// call.
-	
-	BString result;
 	BString line;
-	BMessage updateMsg(message);
 
 	while (!feof(file)) {
-		if (cancel != NULL && *cancel == true)
+		if (lineReader->IsCanceled())
-			break;
+			return B_CANCELED;
-		
+
 		unsigned char c = fgetc(file);
-		
+			// TODO: fgetc() blocks, is there a way to make it timeout?
-		if (c != 255) {
+
+		if (c != 255)
 			line << (char)c;
-			result << (char)c;
+
-		}
-		
 		if (c == '\n') {
-			updateMsg.RemoveName(stringFieldName.String());
+			status_t ret = lineReader->ReadLine(line);
-			updateMsg.AddString(stringFieldName.String(), line);
+			if (ret != B_OK)
-			target.SendMessage(&updateMsg);
+				return ret;
 			line = "";
 		}
 	}
-	
+
-	return result;
+	return B_OK;
 }
 
 
-BCommandPipe&
+BString
-BCommandPipe::operator<<(const char* _arg)
+BCommandPipe::ReadLines(FILE* file)
 {
-	AddArg(_arg);
+	class AllLinesReader : public LineReader {
+	public:
+		AllLinesReader()
+			:
+			fResult("")
+		{
+		}
+
+		virtual bool IsCanceled()
+		{
+			return false;
+		}
+
+		virtual status_t ReadLine(const BString& line)
+		{
+			int lineLength = line.Length();
+			int resultLength = fResult.Length();
+			fResult << line;
+			if (fResult.Length() != lineLength + resultLength)
+				return B_NO_MEMORY;
+			return B_OK;
+		}
+
+		BString Result() const
+		{
+			return fResult;
+		}
+
+	private:
+		BString fResult;
+	} lineReader;
+
+	ReadLines(file, &lineReader);
+
+	return lineReader.Result();
+}
+
+
+// #pragma mark -
+
+
+BCommandPipe&
+BCommandPipe::operator<<(const char* arg)
+{
+	AddArg(arg);
 	return *this;
 }
 
 
 BCommandPipe&
-BCommandPipe::operator<<(const BString& _arg)
+BCommandPipe::operator<<(const BString& arg)
 {
-	AddArg(_arg.String());
+	AddArg(arg.String());
 	return *this;
 }
 
 
 BCommandPipe&
-BCommandPipe::operator<<(const BCommandPipe& _arg)
+BCommandPipe::operator<<(const BCommandPipe& arg)
 {
 	int32 argc;
-	const char** argv = _arg.Argv(argc);
+	const char** argv = arg.Argv(argc);
 	for (int32 i = 0; i < argc; i++)
 		AddArg(argv[i]);
-	
+
 	return *this;
 }