From 11c0b939b4986376b80ec090b273ac8b326aec16 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Mon, 12 Oct 2020 10:44:43 +0200 Subject: [PATCH] kernel: remove output stream stack Instead of pre-allocating a fixed number of TypOutputFile instances on each thread, we allocate the required storage dynamically on the stack. This arguably makes it easier to reason about the global state of GAP. It also enables future simplifications and improvements, e.g. to fix issues were switching between stdout and errout breaks tracking of the output state, which leads to GAP expecting resp. inserting linebreaks in the wrong spots. --- src/common.h | 10 ++++ src/compiler.c | 14 ++--- src/error.c | 17 +++--- src/error.h | 2 +- src/gap.c | 7 +-- src/io.c | 137 +++++++++++++++---------------------------------- src/io.h | 53 ++++++++++++++++--- src/scanner.c | 5 +- src/streams.c | 27 ++++++---- 9 files changed, 134 insertions(+), 138 deletions(-) diff --git a/src/common.h b/src/common.h index e66340feca5..438f2c09c86 100644 --- a/src/common.h +++ b/src/common.h @@ -210,4 +210,14 @@ typedef const struct init_info StructInitInfo; typedef struct TypInputFile TypInputFile; +/**************************************************************************** +** +*T TypOutputFile . . . . . . . . . . structure of an open output file, local +** +** This is a forward declaration so that TypOutputFiles can be used in +** header files. The actual declaration is in io.h. +*/ +typedef struct TypOutputFile TypOutputFile; + + #endif // GAP_COMMON_H diff --git a/src/compiler.c b/src/compiler.c index 5b216475425..8a39f2e7bbb 100644 --- a/src/compiler.c +++ b/src/compiler.c @@ -5190,21 +5190,17 @@ static void CompFunc(Obj func) /**************************************************************************** ** -*F CompileFunc( , , , , ) . . . compile +*F CompileFunc( , , , , ) . . compile */ -Int CompileFunc ( - Obj output, - Obj func, - Obj name, - Int magic1, - Obj magic2 ) +Int CompileFunc(Obj filename, Obj func, Obj name, Int magic1, Obj magic2) { Int i; /* loop variable */ UInt col; UInt compFunctionsNr; /* open the output file */ - if (!OpenOutput(CONST_CSTR_STRING(output), FALSE)) { + TypOutputFile output = { 0 }; + if (!OpenOutput(&output, CONST_CSTR_STRING(filename), FALSE)) { return 0; } col = SyNrCols; @@ -5371,7 +5367,7 @@ Int CompileFunc ( /* close the output file */ SyNrCols = col; - CloseOutput(); + CloseOutput(&output); return compFunctionsNr; } diff --git a/src/error.c b/src/error.c index 0715d99ccb0..f8bcb9e03e0 100644 --- a/src/error.c +++ b/src/error.c @@ -55,18 +55,18 @@ static Obj IsOutputStream; ** ERROR_OUTPUT global variable defined in ** error.g, or "*errout*" otherwise */ -UInt OpenErrorOutput( void ) +UInt OpenErrorOutput(TypOutputFile * output) { /* Try to print the output to stream. Use *errout* as a fallback. */ UInt ret = 0; if (ERROR_OUTPUT != NULL) { if (IsStringConv(ERROR_OUTPUT)) { - ret = OpenOutput(CONST_CSTR_STRING(ERROR_OUTPUT), FALSE); + ret = OpenOutput(output, CONST_CSTR_STRING(ERROR_OUTPUT), FALSE); } else { if (CALL_1ARGS(IsOutputStream, ERROR_OUTPUT) == True) { - ret = OpenOutputStream(ERROR_OUTPUT); + ret = OpenOutputStream(output, ERROR_OUTPUT); } } } @@ -75,7 +75,7 @@ UInt OpenErrorOutput( void ) /* It may be we already tried and failed to open *errout* above but * but this is an extreme case so it can't hurt to try again * anyways */ - ret = OpenOutput("*errout*", FALSE); + ret = OpenOutput(output, "*errout*", FALSE); if (ret) { Pr("failed to open error stream\n", 0, 0); } @@ -173,10 +173,11 @@ static Obj FuncPRINT_CURRENT_STATEMENT(Obj self, Obj stream, Obj context) /* HACK: we want to redirect output */ /* Try to print the output to stream. Use *errout* as a fallback. */ + TypOutputFile output = { 0 }; if ((IsStringConv(stream) && - !OpenOutput(CONST_CSTR_STRING(stream), FALSE)) || - (!IS_STRING(stream) && !OpenOutputStream(stream))) { - if (OpenOutput("*errout*", FALSE)) { + !OpenOutput(&output, CONST_CSTR_STRING(stream), FALSE)) || + (!IS_STRING(stream) && !OpenOutputStream(&output, stream))) { + if (OpenOutput(&output, "*errout*", FALSE)) { Pr("PRINT_CURRENT_STATEMENT: failed to open error stream\n", 0, 0); } else { @@ -216,7 +217,7 @@ static Obj FuncPRINT_CURRENT_STATEMENT(Obj self, Obj stream, Obj context) } /* HACK: close the output again */ - CloseOutput(); + CloseOutput(&output); return 0; } diff --git a/src/error.h b/src/error.h index dedd3fa6e90..e7879e19fda 100644 --- a/src/error.h +++ b/src/error.h @@ -40,7 +40,7 @@ Int RegisterBreakloopObserver(intfunc func); ** ERROR_OUTPUT global variable defined in ** error.g, or "*errout*" otherwise */ -UInt OpenErrorOutput(void); +UInt OpenErrorOutput(TypOutputFile * output); /**************************************************************************** ** diff --git a/src/gap.c b/src/gap.c index cb31707397a..baa56200cff 100644 --- a/src/gap.c +++ b/src/gap.c @@ -194,13 +194,14 @@ static Obj Shell(Obj context, } /* read-eval-print loop */ - if (!OpenOutput(outFile, FALSE)) + TypOutputFile output = { 0 }; + if (!OpenOutput(&output, outFile, FALSE)) ErrorQuit("SHELL: can't open outfile %s",(Int)outFile,0); TypInputFile input = { 0 }; if (!OpenInput(&input, inFile)) { - CloseOutput(); + CloseOutput(&output); ErrorQuit("SHELL: can't open infile %s",(Int)inFile,0); } @@ -317,7 +318,7 @@ static Obj Shell(Obj context, SetPrintObjState(oldPrintObjState); CloseInput(&input); - CloseOutput(); + CloseOutput(&output); STATE(ErrorLLevel) = oldErrorLLevel; SetRecursionDepth(oldRecursionDepth); diff --git a/src/io.c b/src/io.c index 92698dcf3d3..a16cc339ce1 100644 --- a/src/io.c +++ b/src/io.c @@ -44,34 +44,6 @@ #include -/**************************************************************************** -** -*T TypOutputFiles . . . . . . . . . structure of an open output file, local -** -** 'TypOutputFile' describes the information stored for open output files: -** 'file' holds the file identifier which is received from 'SyFopen' and -** which is passed to 'SyFputs' and 'SyFclose' to identify this file. -** 'line' is a buffer that holds the current output line. -** 'pos' is the position of the current character on that line. -*/ -/* the maximal number of used line break hints */ -#define MAXHINTS 100 -typedef struct { - BOOL isstream; - BOOL isstringstream; - Obj stream; - Int file; - - char line[MAXLENOUTPUTLINE]; - Int pos; - BOOL format; - Int indent; - - /* each hint is a tripel (position, value, indent) */ - Int hints[3 * MAXHINTS + 1]; -} TypOutputFile; - - static Char GetLine(TypInputFile * input); static void PutLine2(TypOutputFile * output, const Char * line, UInt len); @@ -98,10 +70,6 @@ enum { struct IOModuleState { - // The stack of open output files - TypOutputFile * OutputStack[MAX_OPEN_FILES]; - int OutputStackPointer; - // A pointer to the current input file TypInputFile * Input; @@ -126,6 +94,9 @@ struct IOModuleState { TypOutputFile InputLogFileOrStream; TypOutputFile OutputLogFileOrStream; + TypOutputFile DefaultOutput; + + Int NoSplitLine; }; @@ -135,7 +106,7 @@ extern inline struct IOModuleState * IO(void) return (struct IOModuleState *)StateSlotsAtOffset(IOStateOffset); } -void LockCurrentOutput(Int lock) +void LockCurrentOutput(BOOL lock) { IO()->IgnoreStdoutErrout = lock ? IO()->Output : NULL; } @@ -303,23 +274,6 @@ Obj GetCachedFilename(UInt id) *F * * * * * * * * * * * open input/output functions * * * * * * * * * * * * */ -#if !defined(HPCGAP) -static TypOutputFile OutputFiles[MAX_OPEN_FILES]; -#endif - -static TypOutputFile * PushNewOutput(void) -{ - GAP_ASSERT(IO()->OutputStackPointer < MAX_OPEN_FILES); - const int sp = IO()->OutputStackPointer++; -#ifdef HPCGAP - if (!IO()->OutputStack[sp]) { - IO()->OutputStack[sp] = AllocateMemoryBlock(sizeof(TypOutputFile)); - } -#endif - GAP_ASSERT(IO()->OutputStack[sp]); - return IO()->OutputStack[sp]; -} - #ifdef HPCGAP static GVarDescriptor DEFAULT_INPUT_STREAM; static GVarDescriptor DEFAULT_OUTPUT_STREAM; @@ -342,22 +296,22 @@ static UInt OpenDefaultInput(TypInputFile * input) return OpenInputStream(input, stream, FALSE); } -static UInt OpenDefaultOutput(void) +static UInt OpenDefaultOutput(TypOutputFile * output) { Obj func, stream; stream = TLS(DefaultOutput); if (stream) - return OpenOutputStream(stream); + return OpenOutputStream(output, stream); func = GVarOptFunction(&DEFAULT_OUTPUT_STREAM); if (!func) - return OpenOutput("*stdout*", FALSE); + return OpenOutput(output, "*stdout*", FALSE); stream = CALL_0ARGS(func); if (!stream) ErrorQuit("DEFAULT_OUTPUT_STREAM() did not return a stream", 0, 0); if (IsStringConv(stream)) - return OpenOutput(CONST_CSTR_STRING(stream), FALSE); + return OpenOutput(output, CONST_CSTR_STRING(stream), FALSE); TLS(DefaultOutput) = stream; - return OpenOutputStream(stream); + return OpenOutputStream(output, stream); } #endif @@ -508,7 +462,8 @@ UInt CloseInput(TypInputFile * input) IO()->Input = input->prev; // don't keep GAP objects alive unnecessarily - memset(input, 0, sizeof(TypInputFile)); + input->stream = 0; + input->sline = 0; return 1; } @@ -843,24 +798,22 @@ UInt CloseOutputLog ( void ) ** If is set to true, then 'OpenOutput' does not truncate the file ** to size 0 if it exists. */ -UInt OpenOutput(const Char * filename, BOOL append) +UInt OpenOutput(TypOutputFile * output, const Char * filename, BOOL append) { + GAP_ASSERT(output); + // do nothing for stdout and errout if caught if (IO()->Output != NULL && IO()->IgnoreStdoutErrout == IO()->Output && (streq(filename, "*errout*") || streq(filename, "*stdout*"))) { return 1; } - /* fail if we can not handle another open output file */ - if (IO()->OutputStackPointer == MAX_OPEN_FILES) - return 0; - #ifdef HPCGAP /* Handle *defout* specially; also, redirect *errout* if we already * have a default channel open. */ if (streq(filename, "*defout*") || (streq(filename, "*errout*") && TLS(threadID) != 0)) - return OpenDefaultOutput(); + return OpenDefaultOutput(output); #endif /* try to open the file */ @@ -869,7 +822,8 @@ UInt OpenOutput(const Char * filename, BOOL append) return 0; /* put the file on the stack, start at position 0 on an empty line */ - TypOutputFile * output = IO()->Output = PushNewOutput(); + output->prev = IO()->Output; + IO()->Output = output; output->isstream = FALSE; output->file = file; output->line[0] = '\0'; @@ -893,15 +847,13 @@ UInt OpenOutput(const Char * filename, BOOL append) */ -UInt OpenOutputStream ( - Obj stream ) +UInt OpenOutputStream(TypOutputFile * output, Obj stream) { - /* fail if we can not handle another open output file */ - if (IO()->OutputStackPointer == MAX_OPEN_FILES) - return 0; + GAP_ASSERT(output); /* put the file on the stack, start at position 0 on an empty line */ - TypOutputFile * output = IO()->Output = PushNewOutput(); + output->prev = IO()->Output; + IO()->Output = output; output->isstream = TRUE; output->isstringstream = (CALL_1ARGS(IsStringStream, stream) == True); output->stream = stream; @@ -935,23 +887,25 @@ UInt OpenOutputStream ( ** On the other hand if you forget to call 'CloseOutput' at the end of a ** 'PrintTo' call or an error will not yield much better results. */ -UInt CloseOutput ( void ) +UInt CloseOutput(TypOutputFile * output) { - TypOutputFile * output = IO()->Output; + GAP_ASSERT(output); // silently refuse to close the test output file; this is probably an // attempt to close *errout* which is silently not opened, so let's // silently not close it - if (IO()->IgnoreStdoutErrout == output) + if (IO()->IgnoreStdoutErrout == IO()->Output) return 1; + GAP_ASSERT(output == IO()->Output); + /* refuse to close the initial output file '*stdout*' */ #ifdef HPCGAP - if (IO()->OutputStackPointer <= 1 && output->isstream && + if (output->prev == 0 && output->isstream && TLS(DefaultOutput) == output->stream) return 0; #else - if (IO()->OutputStackPointer <= 1) + if (output->prev == 0) return 0; #endif @@ -962,8 +916,10 @@ UInt CloseOutput ( void ) } /* revert to previous output file and indicate success */ - const int sp = --IO()->OutputStackPointer; - IO()->Output = sp ? IO()->OutputStack[sp - 1] : 0; + IO()->Output = output->prev; + + // don't keep GAP objects alive unnecessarily + output->stream = 0; return 1; } @@ -1872,7 +1828,7 @@ void Pr ( { #ifdef HPCGAP if (!IO()->Output) { - OpenDefaultOutput(); + OpenDefaultOutput(&IO()->DefaultOutput); } #endif PrTo(IO()->Output, format, arg1, arg2); @@ -1915,7 +1871,12 @@ static Obj FuncINPUT_LINENUMBER(Obj self) static Obj FuncSET_PRINT_FORMATTING_STDOUT(Obj self, Obj val) { - IO()->OutputStack[1]->format = (val != False); + TypOutputFile * output = IO()->Output; + if (!output) + ErrorMayQuit("SET_PRINT_FORMATTING_STDOUT called while no output is opened\n", 0, 0); + while (output->prev) + output = output->prev; + output->format = (val != False); return val; } @@ -1975,10 +1936,6 @@ static Int InitLibrary ( return 0; } -#if !defined(HPCGAP) -static Char OutputFilesStreamCookie[MAX_OPEN_FILES][9]; -#endif - static Int InitKernel ( StructInitInfo * module ) { @@ -1987,13 +1944,7 @@ static Int InitKernel ( IO()->InputLog = 0; IO()->OutputLog = 0; -#if !defined(HPCGAP) - for (Int i = 0; i < MAX_OPEN_FILES; i++) { - IO()->OutputStack[i] = &OutputFiles[i]; - } -#endif - - OpenOutput("*stdout*", FALSE); + OpenOutput(&IO()->DefaultOutput, "*stdout*", FALSE); InitGlobalBag( &FilenameCache, "FilenameCache" ); @@ -2003,14 +1954,6 @@ static Int InitKernel ( DeclareGVar(&DEFAULT_OUTPUT_STREAM, "DEFAULT_OUTPUT_STREAM"); #else - // Initialize cookies for streams. For HPC-GAP we don't need the cookies - // anymore, since the data got moved to thread-local storage. - for (Int i = 0; i < MAX_OPEN_FILES; i++) { - strxcpy(OutputFilesStreamCookie[i], "ostream0", sizeof(OutputFilesStreamCookie[i])); - OutputFilesStreamCookie[i][7] = '0' + i; - InitGlobalBag(&(OutputFiles[i].stream), &(OutputFilesStreamCookie[i][0])); - } - /* tell GASMAN about the global bags */ InitGlobalBag(&(IO()->InputLogFileOrStream.stream), "src/io.c:InputLogFileOrStream"); diff --git a/src/io.h b/src/io.h index d5fdc46a04b..278541b07ed 100644 --- a/src/io.h +++ b/src/io.h @@ -78,6 +78,47 @@ struct TypInputFile { }; +/**************************************************************************** +** +*/ +enum { + // the maximal number of used line break hints + MAXHINTS = 100, + + // the widest allowed screen width + MAXLENOUTPUTLINE = 4096, +}; + + +/**************************************************************************** +** +*T TypOutputFile . . . . . . . . . . structure of an open output file, local +** +** 'TypOutputFile' describes the information stored for open output files: +** 'file' holds the file identifier which is received from 'SyFopen' and +** which is passed to 'SyFputs' and 'SyFclose' to identify this file. +** 'line' is a buffer that holds the current output line. +** 'pos' is the position of the current character on that line. +*/ +struct TypOutputFile { + // pointer to the previously active output + struct TypOutputFile * prev; + + BOOL isstream; + BOOL isstringstream; + Obj stream; + Int file; + + char line[MAXLENOUTPUTLINE]; + Int pos; + BOOL format; + Int indent; + + /* each hint is a tripel (position, value, indent) */ + Int hints[3 * MAXHINTS + 1]; +}; + + /**************************************************************************** ** *F * * * * * * * * * * * open input/output functions * * * * * * * * * * * * @@ -334,7 +375,7 @@ UInt CloseOutputLog(void); ** If is set to true, then 'OpenOutput' does not truncate the file ** to size 0 if it exists. */ -UInt OpenOutput(const Char * filename, BOOL append); +UInt OpenOutput(TypOutputFile * output, const Char * filename, BOOL append); /**************************************************************************** @@ -343,7 +384,7 @@ UInt OpenOutput(const Char * filename, BOOL append); ** ** The same as 'OpenOutput' but for streams. */ -UInt OpenOutputStream(Obj stream); +UInt OpenOutputStream(TypOutputFile * output, Obj stream); /**************************************************************************** @@ -363,7 +404,7 @@ UInt OpenOutputStream(Obj stream); ** On the other hand if you forget to call 'CloseOutput' at the end of a ** 'PrintTo' call or an error will not yield much better results. */ -UInt CloseOutput(void); +UInt CloseOutput(TypOutputFile * output); TypInputFile * GetCurrentInput(void); @@ -396,10 +437,6 @@ UInt GetInputFilenameID(TypInputFile * input); Obj GetCachedFilename(UInt id); -/* the widest allowed screen width */ -#define MAXLENOUTPUTLINE 4096 - - // Reset the indentation level of the current output to zero. The indentation // level can be modified via the '%>' and '%<' formats of 'Pr' resp. 'PrTo'. void ResetOutputIndent(void); @@ -410,7 +447,7 @@ void ResetOutputIndent(void); // // This is used to allow the 'Test' function of the GAP library to // consistently capture all output during testing, see 'FuncREAD_STREAM_LOOP'. -void LockCurrentOutput(Int lock); +void LockCurrentOutput(BOOL lock); /**************************************************************************** ** diff --git a/src/scanner.c b/src/scanner.c index 0d066d9d410..d51fe444979 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -47,7 +47,8 @@ static void SyntaxErrorOrWarning(ScannerState * s, if (STATE(NrErrLine) == 0) { // open error output - OpenErrorOutput(); + TypOutputFile output = { 0 }; + OpenErrorOutput(&output); // print the message ... if (error) @@ -97,7 +98,7 @@ static void SyntaxErrorOrWarning(ScannerState * s, } // close error output - CloseOutput(); + CloseOutput(&output); } if (error) { diff --git a/src/streams.c b/src/streams.c index e37f790055d..61f3ede5e8e 100644 --- a/src/streams.c +++ b/src/streams.c @@ -181,7 +181,8 @@ Obj READ_ALL_COMMANDS(Obj instream, Obj echo, Obj capture, Obj resultCallback) outstream = DoOperation2Args(ValGVar(GVarName("OutputTextString")), outstreamString, True); } - if (outstream && !OpenOutputStream(outstream)) { + TypOutputFile output = { 0 }; + if (outstream && !OpenOutputStream(&output, outstream)) { CloseInput(&input); return Fail; } @@ -226,9 +227,12 @@ Obj READ_ALL_COMMANDS(Obj instream, Obj echo, Obj capture, Obj resultCallback) } } } while (!(status & (STATUS_EOF | STATUS_QUIT | STATUS_QQUIT))); + // FIXME: the above should be in a big GAP_TRY so that we can CloseOutput + // before we rethrow + // FIXME: actually how does that work right now?!? does it?!? if (outstream) - CloseOutput(); + CloseOutput(&output); CloseInput(&input); ClearError(); @@ -733,10 +737,12 @@ static Obj PRINT_OR_APPEND_TO_FILE_OR_STREAM(Obj args, int append, int file) /* first entry is the file or stream */ destination = ELM_LIST(args, 1); + TypOutputFile output = { 0 }; + /* try to open the output and handle failures */ if (file) { RequireStringRep(funcname, destination); - i = OpenOutput(CONST_CSTR_STRING(destination), append); + i = OpenOutput(&output, CONST_CSTR_STRING(destination), append); if (!i) { if (streq(CSTR_STRING(destination), "*errout*")) { Panic("Failed to open *errout*!"); @@ -750,7 +756,7 @@ static Obj PRINT_OR_APPEND_TO_FILE_OR_STREAM(Obj args, int append, int file) ErrorQuit("%s: must be an output stream", (Int)funcname, 0); } - i = OpenOutputStream(destination); + i = OpenOutputStream(&output, destination); if (!i) { ErrorQuit("%s: cannot open stream for output", (Int)funcname, 0); } @@ -775,13 +781,13 @@ static Obj PRINT_OR_APPEND_TO_FILE_OR_STREAM(Obj args, int append, int file) } GAP_CATCH { - CloseOutput(); + CloseOutput(&output); GAP_THROW(); } } /* close the output file again, and return nothing */ - if ( ! CloseOutput() ) { + if (!CloseOutput(&output)) { ErrorQuit("%s: cannot close output", (Int)funcname, 0); } @@ -915,20 +921,21 @@ static Obj FuncREAD_STREAM_LOOP_WITH_CONTEXT(Obj self, return False; } - if (!OpenOutputStream(outstream)) { + TypOutputFile output = { 0 }; + if (!OpenOutputStream(&output, outstream)) { res = CloseInput(&input); GAP_ASSERT(res); return False; } - LockCurrentOutput(1); + LockCurrentOutput(TRUE); READ_TEST_OR_LOOP(context, &input); - LockCurrentOutput(0); + LockCurrentOutput(FALSE); res = CloseInput(&input); GAP_ASSERT(res); - res &= CloseOutput(); + res &= CloseOutput(&output); GAP_ASSERT(res); return res ? True : False;