From 8dadca32b1d802c2de009a131ed3b6fa01738655 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Wed, 2 Sep 2009 14:52:13 +0000 Subject: [PATCH] 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 | 67 +++--- src/kits/shared/CommandPipe.cpp | 319 +++++++++++++++------------ 2 files changed, 219 insertions(+), 167 deletions(-) diff --git a/headers/private/shared/CommandPipe.h b/headers/private/shared/CommandPipe.h index 3be0ac8f83..9d6faff2d7 100644 --- a/headers/private/shared/CommandPipe.h +++ b/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! - thread_id Pipe(int* outdes, int* errdes) const; - thread_id Pipe(int* outdes) const; - thread_id PipeAll(int* outAndErrDes) const; - + // (stdOut/stdErr) when you are done with them! + thread_id Pipe(int* stdOut, int* stdErr) const; + thread_id Pipe(int* stdOut) 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 - // when "*cancel" is true. It reports each line it has read to "target" - // using the supplied "_message" and string field name. "cancel" can be - // NULL - BString ReadLines(FILE* file, bool* cancel, - BMessenger& target, const BMessage& message, - const BString& stringFieldName); - - // You need NOT free/delete the return array of strings - const char** Argv(int32& _argc) const; - + + // Reading the Pipe output + + class LineReader { + public: + virtual bool IsCanceled() = 0; + virtual status_t ReadLine(const BString& line) = 0; + // TODO: Add a Timeout() method. + }; + + // 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 fErrDes[2]; - bool fOutDesOpen; - bool fErrDesOpen; + int fStdOut[2]; + int fStdErr[2]; + bool fStdOutOpen; + bool fStdErrOpen; }; } // namespace BPrivate diff --git a/src/kits/shared/CommandPipe.cpp b/src/kits/shared/CommandPipe.cpp index 6a2a9aa497..b0b1d129ea 100644 --- a/src/kits/shared/CommandPipe.cpp +++ b/src/kits/shared/CommandPipe.cpp @@ -4,6 +4,7 @@ * * Authors: * Ramshankar, v.ramshankar@gmail.com + * Stephan Aßmus */ //! 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(strdup(arg))) == true ? - B_OK : B_ERROR); + if (arg == NULL || arg[0] == '\0') + return B_BAD_VALUE; + + char* argCopy = strdup(arg); + if (argCopy == NULL) + return B_NO_MEMORY; + + if (!fArgList.AddItem(reinterpret_cast(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(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++) - free(fArgList.RemoveItem(0L)); - + for (int32 i = fArgList.CountItems() - 1; i >= 0; i--) + free(fArgList.ItemAtFast(i)); fArgList.MakeEmpty(); + Close(); } @@ -65,29 +78,29 @@ BCommandPipe::FlushArgs() void BCommandPipe::Close() { - if (fErrDesOpen) { - close(fErrDes[0]); - fErrDesOpen = false; + if (fStdErrOpen) { + close(fStdErr[0]); + fStdErrOpen = false; } - - if (fOutDesOpen) { - close(fOutDes[0]); - fOutDesOpen = false; + + if (fStdOutOpen) { + close(fStdOut[0]); + fStdOutOpen = false; } } const char** -BCommandPipe::Argv(int32& _argc) const +BCommandPipe::Argv(int32& argc) const { - // *** Warning *** Freeing is left to caller!! Indicated in Header - int32 argc = fArgList.CountItems(); - const char **argv = (const char**)malloc((argc + 1) * sizeof(char*)); + // NOTE: Freeing is left to caller as indicated in the header! + argc = fArgList.CountItems(); + const char** argv = reinterpret_cast( + malloc((argc + 1) * sizeof(char*))); for (int32 i = 0; i < argc; i++) - argv[i] = (const char*)fArgList.ItemAtFast(i); - + argv[i] = reinterpret_cast(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) - int oldstdout; - int oldstderr; - pipe(outAndErrDes); - oldstdout = dup(STDOUT_FILENO); - oldstderr = dup(STDERR_FILENO); + // (stdOut) + int oldStdOut; + int oldStdErr; + pipe(stdOutAndErr); + oldStdOut = dup(STDOUT_FILENO); + oldStdErr = dup(STDERR_FILENO); close(STDOUT_FILENO); close(STDERR_FILENO); - dup2(outAndErrDes[1], STDOUT_FILENO); - dup2(outAndErrDes[1], STDERR_FILENO); - + // TODO: This looks broken, using "stdOutAndErr[1]" twice! + dup2(stdOutAndErr[1], STDOUT_FILENO); + dup2(stdOutAndErr[1], STDERR_FILENO); + // Construct the argv vector - int32 argc = fArgList.CountItems(); - const char **argv = (const char**)malloc((argc + 1) * sizeof(char*)); - for (int32 i = 0; i < argc; i++) - argv[i] = (const char*)fArgList.ItemAtFast(i); - - argv[argc] = NULL; - + int32 argc; + const char** argv = Argv(argc); + // Load the app image... and pass the args - thread_id appThread = load_image((int)argc, argv, const_cast< - const char**>(environ)); + thread_id appThread = load_image((int)argc, argv, + const_cast(environ)); - dup2(oldstdout, STDOUT_FILENO); - dup2(oldstderr, STDERR_FILENO); - close(oldstdout); - close(oldstderr); + dup2(oldStdOut, STDOUT_FILENO); + dup2(oldStdErr, STDERR_FILENO); + close(oldStdOut); + 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 oldstderr; - pipe(outdes); - pipe(errdes); - oldstdout = dup(STDOUT_FILENO); - oldstderr = dup(STDERR_FILENO); + int oldStdOut; + int oldStdErr; + pipe(stdOut); + pipe(stdErr); + oldStdOut = dup(STDOUT_FILENO); + oldStdErr = dup(STDERR_FILENO); close(STDOUT_FILENO); close(STDERR_FILENO); - dup2(outdes[1], STDOUT_FILENO); - dup2(errdes[1], STDERR_FILENO); - + dup2(stdOut[1], STDOUT_FILENO); + dup2(stdErr[1], STDERR_FILENO); + // Construct the argv vector - int32 argc = fArgList.CountItems(); - const char **argv = (const char**)malloc((argc + 1) * sizeof(char*)); - for (int32 i = 0; i < argc; i++) - argv[i] = (const char*)fArgList.ItemAtFast(i); - - argv[argc] = NULL; - + int32 argc; + const char** argv = Argv(argc); + // 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(oldstderr, STDERR_FILENO); - close(oldstdout); - close(oldstderr); - - delete[] argv; + dup2(oldStdOut, STDOUT_FILENO); + dup2(oldStdErr, STDERR_FILENO); + close(oldStdOut); + close(oldStdErr); + + 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]; - thread_id tid = Pipe(outdes, errdes); - close(errdes[0]); - close(errdes[1]); + int stdErr[2]; + thread_id tid = Pipe(stdOut, stdErr); + close(stdErr[0]); + 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"); - *_err = fdopen(fErrDes[0], "r"); - + thread_id tid = Pipe(fStdOut, fStdErr); + 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); - - if (tid == B_ERROR || tid == B_NO_MEMORY) - return tid; - - resume_thread(tid); - - close(fOutDes[1]); - fOutDesOpen = true; - - *_outAndErr = fdopen(fOutDes[0], "r"); + + thread_id tid = PipeAll(fStdOut); + if (tid >= 0) + resume_thread(tid); + + close(fStdOut[1]); + fStdOutOpen = true; + + *_outAndErr = fdopen(fStdOut[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(errdes[0]); - close(outdes[1]); - close(errdes[1]); + close(stdOut[0]); + close(stdErr[0]); + close(stdOut[1]); + close(stdErr[1]); } @@ -256,69 +261,107 @@ BCommandPipe::RunAsync() // #pragma mark - -BString -BCommandPipe::ReadLines(FILE* file, bool* cancel, BMessenger& target, - const BMessage& message, const BString& stringFieldName) +status_t +BCommandPipe::ReadLines(FILE* file, LineReader* lineReader) { - // Reads output of file, line by line. The entire output is returned - // and as each line is being read "target" (if any) is informed, - // with "message" i.e. AddString (stringFieldName, ) - - // "cancel" cancels the reading process, when it becomes true (unless its - // waiting on fgetc()) and I don't know how to cancel the waiting fgetc() - // call. - - BString result; + // Reads output of file, line by line. Each line is passed to lineReader + // for inspection, and the IsCanceled() method is repeatedly called. + + if (file == NULL || lineReader == NULL) + return B_BAD_VALUE; + BString line; - BMessage updateMsg(message); while (!feof(file)) { - if (cancel != NULL && *cancel == true) - break; - + if (lineReader->IsCanceled()) + return B_CANCELED; + unsigned char c = fgetc(file); - - if (c != 255) { + // TODO: fgetc() blocks, is there a way to make it timeout? + + if (c != 255) line << (char)c; - result << (char)c; - } - + if (c == '\n') { - updateMsg.RemoveName(stringFieldName.String()); - updateMsg.AddString(stringFieldName.String(), line); - target.SendMessage(&updateMsg); + status_t ret = lineReader->ReadLine(line); + if (ret != B_OK) + return ret; line = ""; } } - - return result; + + return B_OK; } -BCommandPipe& -BCommandPipe::operator<<(const char* _arg) +BString +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; }