From a68bebf4a288a0a75edd573b924d9f49b723e834 Mon Sep 17 00:00:00 2001 From: Sylvie <35663410+Rangi42@users.noreply.github.com> Date: Wed, 27 Mar 2024 10:42:53 -0400 Subject: [PATCH] Use a `Defer` struct to close files and restore lexer state with RAII (#1379) --- include/helpers.hpp | 8 ++ src/asm/fstack.cpp | 1 - src/asm/lexer.cpp | 166 ++++++++++++++++-------------------------- src/asm/output.cpp | 5 +- src/asm/section.cpp | 26 +++---- src/fix/main.cpp | 49 ++++++------- src/gfx/process.cpp | 4 +- src/link/assign.cpp | 2 +- src/link/main.cpp | 2 +- src/link/object.cpp | 4 +- src/link/output.cpp | 19 ++--- src/link/script.y | 7 +- src/link/sdas_obj.cpp | 10 +-- 13 files changed, 131 insertions(+), 172 deletions(-) diff --git a/include/helpers.hpp b/include/helpers.hpp index 3782c363..010beb5a 100644 --- a/include/helpers.hpp +++ b/include/helpers.hpp @@ -86,4 +86,12 @@ static inline int clz(unsigned int x) { // For lack of , this adds some more brevity #define RANGE(s) std::begin(s), std::end(s) +// For ad-hoc RAII in place of a `defer` statement or cross-platform `__attribute__((cleanup))` +template +struct Defer { + T deferred; + Defer(T func) : deferred(func) {} + ~Defer() { deferred(); } +}; + #endif // HELPERS_H diff --git a/src/asm/fstack.cpp b/src/asm/fstack.cpp index 07833cd8..3cc0506c 100644 --- a/src/asm/fstack.cpp +++ b/src/asm/fstack.cpp @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: MIT */ #include "asm/fstack.hpp" - #include #include diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index cdcd2912..a45d1db4 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -90,6 +90,8 @@ static void mapFile(void *&mappingAddr, int fd, std::string const &path, size_t #endif // !( defined(_MSC_VER) || defined(__MINGW32__) ) +using namespace std::literals; + // Bison 3.6 changed token "types" to "kinds"; cast to int for simple compatibility #define T_(name) (int)yy::parser::token::name @@ -526,9 +528,12 @@ static bool continuesIdentifier(int c); static uint32_t readBracketedMacroArgNum() { bool disableMacroArgs = lexerState->disableMacroArgs; bool disableInterpolation = lexerState->disableInterpolation; - lexerState->disableMacroArgs = false; lexerState->disableInterpolation = false; + Defer restoreExpansions{[&] { + lexerState->disableMacroArgs = disableMacroArgs; + lexerState->disableInterpolation = disableInterpolation; + }}; uint32_t num = 0; int c = peek(); @@ -573,11 +578,9 @@ static uint32_t readBracketedMacroArgNum() { } else if (num == 0 && !symbolError) { error("Invalid bracketed macro argument '\\<0>'\n"); return 0; + } else { + return num; } - - lexerState->disableMacroArgs = disableMacroArgs; - lexerState->disableInterpolation = disableInterpolation; - return num; } static std::shared_ptr readMacroArg(char name) { @@ -817,6 +820,15 @@ static void handleCRLF(int c) { shiftChar(); } +static auto scopedDisableExpansions() { + lexerState->disableMacroArgs = true; + lexerState->disableInterpolation = true; + return Defer{[&] { + lexerState->disableMacroArgs = false; + lexerState->disableInterpolation = false; + }}; +} + // "Services" provided by the lexer to the rest of the program uint32_t lexer_GetLineNo() { @@ -841,15 +853,14 @@ void lexer_DumpStringExpansions() { // Functions to discard non-tokenized characters static void discardBlockComment() { - lexerState->disableMacroArgs = true; - lexerState->disableInterpolation = true; + Defer reenableExpansions = scopedDisableExpansions(); for (;;) { int c = nextChar(); switch (c) { case EOF: error("Unterminated block comment\n"); - goto finish; + return; case '\r': // Handle CRLF before nextLine() since shiftChar updates colNo handleCRLF(c); @@ -866,29 +877,23 @@ static void discardBlockComment() { case '*': if (peek() == '/') { shiftChar(); - goto finish; + return; } // fallthrough default: continue; } } -finish: - lexerState->disableMacroArgs = false; - lexerState->disableInterpolation = false; } static void discardComment() { - lexerState->disableMacroArgs = true; - lexerState->disableInterpolation = true; + Defer reenableExpansions = scopedDisableExpansions(); for (;; shiftChar()) { int c = peek(); if (c == EOF || c == '\r' || c == '\n') break; } - lexerState->disableMacroArgs = false; - lexerState->disableInterpolation = false; } static void discardLineContinuation() { @@ -1229,13 +1234,10 @@ static void appendEscapedSubstring(std::string &yylval, std::string const &str) } static std::string readString(bool raw) { - lexerState->disableMacroArgs = true; - lexerState->disableInterpolation = true; - - std::string yylval; - bool multiline = false; + Defer reenableExpansions = scopedDisableExpansions(); // We reach this function after reading a single quote, but we also support triple quotes + bool multiline = false; if (peek() == '"') { shiftChar(); if (peek() == '"') { @@ -1244,17 +1246,17 @@ static std::string readString(bool raw) { multiline = true; } else { // "" is an empty string, skip the loop - goto finish; + return ""s; } } - for (;;) { + for (std::string yylval = ""s;;) { int c = peek(); // '\r', '\n' or EOF ends a single-line string early if (c == EOF || (!multiline && (c == '\r' || c == '\n'))) { error("Unterminated string\n"); - break; + return yylval; } // We'll be staying in the string, so we can safely consume the char @@ -1281,7 +1283,7 @@ static std::string readString(bool raw) { } shiftChar(); } - goto finish; + return yylval; case '\\': // Character escape or macro arg if (raw) @@ -1364,21 +1366,13 @@ static std::string readString(bool raw) { yylval += c; } - -finish: - lexerState->disableMacroArgs = false; - lexerState->disableInterpolation = false; - - return yylval; } static void appendStringLiteral(std::string &yylval, bool raw) { - lexerState->disableMacroArgs = true; - lexerState->disableInterpolation = true; - - bool multiline = false; + Defer reenableExpansions = scopedDisableExpansions(); // We reach this function after reading a single quote, but we also support triple quotes + bool multiline = false; yylval += '"'; if (peek() == '"') { yylval += '"'; @@ -1390,7 +1384,7 @@ static void appendStringLiteral(std::string &yylval, bool raw) { multiline = true; } else { // "" is an empty string, skip the loop - goto finish; + return; } } @@ -1400,7 +1394,7 @@ static void appendStringLiteral(std::string &yylval, bool raw) { // '\r', '\n' or EOF ends a single-line string early if (c == EOF || (!multiline && (c == '\r' || c == '\n'))) { error("Unterminated string\n"); - break; + return; } // We'll be staying in the string, so we can safely consume the char @@ -1428,7 +1422,7 @@ static void appendStringLiteral(std::string &yylval, bool raw) { shiftChar(); } yylval += '"'; - goto finish; + return; case '\\': // Character escape or macro arg if (raw) @@ -1506,10 +1500,6 @@ static void appendStringLiteral(std::string &yylval, bool raw) { yylval += c; } - -finish: - lexerState->disableMacroArgs = false; - lexerState->disableInterpolation = false; } // Lexer core @@ -1981,12 +1971,11 @@ finish: static Token skipIfBlock(bool toEndc) { lexer_SetMode(LEXER_NORMAL); uint32_t startingDepth = lexer_GetIFDepth(); - Token token; - bool atLineStart = lexerState->atLineStart; - // Prevent expanding macro args and symbol interpolation in this state - lexerState->disableMacroArgs = true; - lexerState->disableInterpolation = true; + bool atLineStart = lexerState->atLineStart; + Defer notAtLineStart{[&] { lexerState->atLineStart = false; }}; + + Defer reenableExpansions = scopedDisableExpansions(); for (;;) { if (atLineStart) { @@ -2000,8 +1989,7 @@ static Token skipIfBlock(bool toEndc) { if (startsIdentifier(c)) { shiftChar(); - token = readIdentifier(c); - switch (token.type) { + switch (Token token = readIdentifier(c); token.type) { case T_(POP_IF): lexer_IncIFDepth(); break; @@ -2010,7 +1998,7 @@ static Token skipIfBlock(bool toEndc) { if (lexer_ReachedELSEBlock()) fatalerror("Found ELIF after an ELSE block\n"); if (!toEndc && lexer_GetIFDepth() == startingDepth) - goto finish; + return token; break; case T_(POP_ELSE): @@ -2018,12 +2006,12 @@ static Token skipIfBlock(bool toEndc) { fatalerror("Found ELSE after an ELSE block\n"); lexer_ReachELSEBlock(); if (!toEndc && lexer_GetIFDepth() == startingDepth) - goto finish; + return token; break; case T_(POP_ENDC): if (lexer_GetIFDepth() == startingDepth) - goto finish; + return token; lexer_DecIFDepth(); break; @@ -2039,8 +2027,7 @@ static Token skipIfBlock(bool toEndc) { int c = nextChar(); if (c == EOF) { - token = Token(T_(YYEOF)); - goto finish; + return Token(T_(YYEOF)); } else if (c == '\\') { // Unconditionally skip the next char, including line continuations c = nextChar(); @@ -2056,13 +2043,6 @@ static Token skipIfBlock(bool toEndc) { } } while (!atLineStart); } - -finish: - lexerState->disableMacroArgs = false; - lexerState->disableInterpolation = false; - lexerState->atLineStart = false; - - return token; } static Token yylex_SKIP_TO_ELIF() { @@ -2076,11 +2056,11 @@ static Token yylex_SKIP_TO_ENDC() { static Token yylex_SKIP_TO_ENDR() { lexer_SetMode(LEXER_NORMAL); int depth = 1; - bool atLineStart = lexerState->atLineStart; - // Prevent expanding macro args and symbol interpolation in this state - lexerState->disableMacroArgs = true; - lexerState->disableInterpolation = true; + bool atLineStart = lexerState->atLineStart; + Defer notAtLineStart{[&] { lexerState->atLineStart = false; }}; + + Defer reenableExpansions = scopedDisableExpansions(); for (;;) { if (atLineStart) { @@ -2104,7 +2084,7 @@ static Token yylex_SKIP_TO_ENDR() { case T_(POP_ENDR): depth--; if (!depth) - goto finish; + return Token(T_(YYEOF)); // yywrap() will finish the REPT/FOR loop break; case T_(POP_IF): @@ -2127,7 +2107,7 @@ static Token yylex_SKIP_TO_ENDR() { int c = nextChar(); if (c == EOF) { - goto finish; + return Token(T_(YYEOF)); } else if (c == '\\') { // Unconditionally skip the next char, including line continuations c = nextChar(); @@ -2143,14 +2123,6 @@ static Token yylex_SKIP_TO_ENDR() { } } while (!atLineStart); } - -finish: - lexerState->disableMacroArgs = false; - lexerState->disableInterpolation = false; - lexerState->atLineStart = false; - - // yywrap() will finish the REPT/FOR loop - return Token(T_(YYEOF)); } yy::parser::symbol_type yylex() { @@ -2195,11 +2167,9 @@ static Capture startCapture() { // The following assertion checks that. assert(lexerState->atLineStart); - assert(!lexerState->capturing); + assert(!lexerState->capturing && lexerState->captureBuf == nullptr); lexerState->capturing = true; lexerState->captureSize = 0; - lexerState->disableMacroArgs = true; - lexerState->disableInterpolation = true; Capture capture = {.lineNo = lexer_GetLineNo(), .body = nullptr, .size = 0}; if (auto *mmap = std::get_if(&lexerState->content); @@ -2231,13 +2201,13 @@ static void endCapture(Capture &capture) { lexerState->capturing = false; lexerState->captureBuf = nullptr; - lexerState->disableMacroArgs = false; - lexerState->disableInterpolation = false; } Capture lexer_CaptureRept() { Capture capture = startCapture(); + Defer reenableExpansions = scopedDisableExpansions(); + size_t depth = 0; int c = EOF; @@ -2258,10 +2228,11 @@ Capture lexer_CaptureRept() { case T_(POP_ENDR): if (!depth) { + endCapture(capture); // The final ENDR has been captured, but we don't want it! // We know we have read exactly "ENDR", not e.g. an EQUS - lexerState->captureSize -= strlen("ENDR"); - goto finish; + capture.size -= strlen("ENDR"); + return capture; } depth--; break; @@ -2275,26 +2246,22 @@ Capture lexer_CaptureRept() { for (;; c = nextChar()) { if (c == EOF) { error("Unterminated REPT/FOR block\n"); - goto finish; + endCapture(capture); + capture.body = nullptr; // Indicates that it reached EOF before an ENDR + return capture; } else if (c == '\n' || c == '\r') { handleCRLF(c); break; } } } - -finish: - endCapture(capture); - - if (c == EOF) - capture.body = nullptr; // Indicates that it reached EOF before an ENDR terminated it - - return capture; } Capture lexer_CaptureMacro() { Capture capture = startCapture(); + Defer reenableExpansions = scopedDisableExpansions(); + // If the file is `mmap`ed, we need not to unmap it to keep access to the macro if (auto *mmap = std::get_if(&lexerState->content); mmap) mmap->isReferenced = true; @@ -2311,10 +2278,11 @@ Capture lexer_CaptureMacro() { if (startsIdentifier(c)) { switch (readIdentifier(c).type) { case T_(POP_ENDM): + endCapture(capture); // The ENDM has been captured, but we don't want it! // We know we have read exactly "ENDM", not e.g. an EQUS - lexerState->captureSize -= strlen("ENDM"); - goto finish; + capture.size -= strlen("ENDM"); + return capture; default: break; @@ -2325,19 +2293,13 @@ Capture lexer_CaptureMacro() { for (;; c = nextChar()) { if (c == EOF) { error("Unterminated macro definition\n"); - goto finish; + endCapture(capture); + capture.body = nullptr; // Indicates that it reached EOF before an ENDM + return capture; } else if (c == '\n' || c == '\r') { handleCRLF(c); break; } } } - -finish: - endCapture(capture); - - if (c == EOF) - capture.body = nullptr; // Indicates that it reached EOF before an ENDM terminated it - - return capture; } diff --git a/src/asm/output.cpp b/src/asm/output.cpp index 618dfc5a..8702c480 100644 --- a/src/asm/output.cpp +++ b/src/asm/output.cpp @@ -12,6 +12,7 @@ #include #include "error.hpp" +#include "helpers.hpp" // Defer #include "asm/fstack.hpp" #include "asm/lexer.hpp" @@ -304,7 +305,6 @@ static void writeFileStackNode(FileStackNode const &node, FILE *file) { // Write an objectfile void out_WriteObject() { FILE *file; - if (objectName != "-") { file = fopen(objectName.c_str(), "wb"); } else { @@ -313,6 +313,7 @@ void out_WriteObject() { } if (!file) err("Failed to open object file '%s'", objectName.c_str()); + Defer closeFile{[&] { fclose(file); }}; // Also write symbols that weren't written above sym_ForEach(registerUnregisteredSymbol); @@ -349,8 +350,6 @@ void out_WriteObject() { for (Assertion &assert : assertions) writeassert(assert, file); - - fclose(file); } // Set the objectfilename diff --git a/src/asm/section.cpp b/src/asm/section.cpp index 1e22221a..b6eb1bda 100644 --- a/src/asm/section.cpp +++ b/src/asm/section.cpp @@ -833,8 +833,9 @@ void sect_BinaryFile(std::string const &name, int32_t startPos) { if (!checkcodesection()) return; - std::optional fullPath = fstk_FindFile(name); - FILE *file = fullPath ? fopen(fullPath->c_str(), "rb") : nullptr; + FILE *file = nullptr; + if (std::optional fullPath = fstk_FindFile(name); fullPath) + file = fopen(fullPath->c_str(), "rb"); if (!file) { if (generatedMissingIncludes) { if (verbose) @@ -845,6 +846,7 @@ void sect_BinaryFile(std::string const &name, int32_t startPos) { error("Error opening INCBIN file '%s': %s\n", name.c_str(), strerror(errno)); return; } + Defer closeFile{[&] { fclose(file); }}; int32_t fsize = -1; int byte; @@ -854,12 +856,12 @@ void sect_BinaryFile(std::string const &name, int32_t startPos) { if (startPos > fsize) { error("Specified start position is greater than length of file\n"); - goto cleanup; + return; } fseek(file, startPos, SEEK_SET); if (!reserveSpace(fsize - startPos)) - goto cleanup; + return; } else { if (errno != ESPIPE) error( @@ -878,9 +880,6 @@ void sect_BinaryFile(std::string const &name, int32_t startPos) { if (ferror(file)) error("Error reading INCBIN file '%s': %s\n", name.c_str(), strerror(errno)); - -cleanup: - fclose(file); } void sect_BinaryFileSlice(std::string const &name, int32_t startPos, int32_t length) { @@ -901,8 +900,9 @@ void sect_BinaryFileSlice(std::string const &name, int32_t startPos, int32_t len if (!reserveSpace(length)) return; - std::optional fullPath = fstk_FindFile(name); - FILE *file = fullPath ? fopen(fullPath->c_str(), "rb") : nullptr; + FILE *file = nullptr; + if (std::optional fullPath = fstk_FindFile(name); fullPath) + file = fopen(fullPath->c_str(), "rb"); if (!file) { if (generatedMissingIncludes) { if (verbose) @@ -913,6 +913,7 @@ void sect_BinaryFileSlice(std::string const &name, int32_t startPos, int32_t len } return; } + Defer closeFile{[&] { fclose(file); }}; int32_t fsize; @@ -921,7 +922,7 @@ void sect_BinaryFileSlice(std::string const &name, int32_t startPos, int32_t len if (startPos > fsize) { error("Specified start position is greater than length of file\n"); - goto cleanup; + return; } if ((startPos + length) > fsize) { @@ -932,7 +933,7 @@ void sect_BinaryFileSlice(std::string const &name, int32_t startPos, int32_t len length, fsize ); - goto cleanup; + return; } fseek(file, startPos, SEEK_SET); @@ -957,9 +958,6 @@ void sect_BinaryFileSlice(std::string const &name, int32_t startPos, int32_t len error("Premature end of file (%" PRId32 " bytes left to read)\n", length + 1); } } - -cleanup: - fclose(file); } // Section stack routines diff --git a/src/fix/main.cpp b/src/fix/main.cpp index f7a2055e..57bab404 100644 --- a/src/fix/main.cpp +++ b/src/fix/main.cpp @@ -1166,47 +1166,44 @@ static void processFile(int input, int output, char const *name, off_t fileSize) static bool processFilename(char const *name) { nbErrors = 0; + if (!strcmp(name, "-")) { (void)setmode(STDIN_FILENO, O_BINARY); (void)setmode(STDOUT_FILENO, O_BINARY); name = ""; processFile(STDIN_FILENO, STDOUT_FILENO, name, 0); - } else { // POSIX specifies that the results of O_RDWR on a FIFO are undefined. // However, this is necessary to avoid a TOCTTOU, if the file was changed between // `stat()` and `open(O_RDWR)`, which could trigger the UB anyway. // Thus, we're going to hope that either the `open` fails, or it succeeds but IO // operations may fail, all of which we handle. - int input = open(name, O_RDWR | O_BINARY); - struct stat stat; - - if (input == -1) { + if (int input = open(name, O_RDWR | O_BINARY); input == -1) { report("FATAL: Failed to open \"%s\" for reading+writing: %s\n", name, strerror(errno)); - goto finish; - } - - if (fstat(input, &stat) == -1) { - report("FATAL: Failed to stat \"%s\": %s\n", name, strerror(errno)); - } else if (!S_ISREG(stat.st_mode)) { // TODO: Do we want to support other types? - report( - "FATAL: \"%s\" is not a regular file, and thus cannot be modified in-place\n", name - ); - } else if (stat.st_size < 0x150) { - // This check is in theory redundant with the one in `processFile`, but it - // prevents passing a file size of 0, which usually indicates pipes - report( - "FATAL: \"%s\" too short, expected at least 336 ($150) bytes, got only %jd\n", - name, - (intmax_t)stat.st_size - ); } else { - processFile(input, input, name, stat.st_size); + Defer closeInput{[&] { close(input); }}; + struct stat stat; + if (fstat(input, &stat) == -1) { + report("FATAL: Failed to stat \"%s\": %s\n", name, strerror(errno)); + } else if (!S_ISREG(stat.st_mode)) { // TODO: Do we want to support other types? + report( + "FATAL: \"%s\" is not a regular file, and thus cannot be modified in-place\n", + name + ); + } else if (stat.st_size < 0x150) { + // This check is in theory redundant with the one in `processFile`, but it + // prevents passing a file size of 0, which usually indicates pipes + report( + "FATAL: \"%s\" too short, expected at least 336 ($150) bytes, got only %jd\n", + name, + (intmax_t)stat.st_size + ); + } else { + processFile(input, input, name, stat.st_size); + } } - - close(input); } -finish: + if (nbErrors) fprintf( stderr, diff --git a/src/gfx/process.cpp b/src/gfx/process.cpp index a3f5d727..20d92035 100644 --- a/src/gfx/process.cpp +++ b/src/gfx/process.cpp @@ -1113,7 +1113,7 @@ void process() { case ProtoPalette::THEY_BIGGER: // Do nothing, they already contain us attrs.protoPaletteID = n; - goto contained; + goto continue_visiting_tiles; // Can't `continue` from within a nested loop case ProtoPalette::NEITHER: break; // Keep going @@ -1139,7 +1139,7 @@ void process() { ); } protoPalettes.push_back(tileColors); -contained:; +continue_visiting_tiles:; } options.verbosePrint( diff --git a/src/link/assign.cpp b/src/link/assign.cpp index 7571e805..8980a91f 100644 --- a/src/link/assign.cpp +++ b/src/link/assign.cpp @@ -393,7 +393,7 @@ void assign_AssignSections() { fprintf(stderr, "%c \"%s\"", nbSections == 0 ? ';' : ',', section->name.c_str()); nbSections++; if (nbSections == 10) - goto max_out; + goto max_out; // Can't `break` out of a nested loop } } diff --git a/src/link/main.cpp b/src/link/main.cpp index 36df692c..8d7366f9 100644 --- a/src/link/main.cpp +++ b/src/link/main.cpp @@ -320,7 +320,7 @@ static void parseScrambleSpec(char const *spec) { argErr('S', "Cannot imply limit for region \"%.*s\"", regionNamePrintLen, regionName); } -next: +next: // Can't `continue` a `for` loop with this nontrivial iteration logic if (spec) { assert(*spec == ',' || *spec == '\0'); if (*spec == ',') diff --git a/src/link/object.cpp b/src/link/object.cpp index 33ac1130..2d878ecb 100644 --- a/src/link/object.cpp +++ b/src/link/object.cpp @@ -473,7 +473,6 @@ static void readAssertion( void obj_ReadFile(char const *fileName, unsigned int fileID) { FILE *file; - if (strcmp(fileName, "-")) { file = fopen(fileName, "rb"); } else { @@ -482,6 +481,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) { } if (!file) err("Failed to open file \"%s\"", fileName); + Defer closeFile{[&] { fclose(file); }}; // First, check if the object is a RGBDS object or a SDCC one. If the first byte is 'R', // we'll assume it's a RGBDS object file, and otherwise, that it's a SDCC object file. @@ -630,8 +630,6 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) { } } } - - fclose(file); } void obj_CheckAssertions() { diff --git a/src/link/output.cpp b/src/link/output.cpp index 09f453b2..ada4aec6 100644 --- a/src/link/output.cpp +++ b/src/link/output.cpp @@ -199,6 +199,10 @@ static void writeROM() { if (!outputFile) err("Failed to open output file \"%s\"", outputFileName); } + Defer closeOutputFile{[&] { + if (outputFile) + fclose(outputFile); + }}; if (overlayFileName) { if (strcmp(overlayFileName, "-")) { @@ -210,6 +214,10 @@ static void writeROM() { if (!overlayFile) err("Failed to open overlay file \"%s\"", overlayFileName); } + Defer closeOverlayFile{[&] { + if (overlayFile) + fclose(overlayFile); + }}; uint32_t nbOverlayBanks = checkOverlaySize(); @@ -230,11 +238,6 @@ static void writeROM() { sectionTypeInfo[SECTTYPE_ROMX].size ); } - - if (outputFile) - fclose(outputFile); - if (overlayFile) - fclose(overlayFile); } // Checks whether this character is legal as the first character of a symbol's name in a sym file @@ -533,6 +536,7 @@ static void writeSym() { } if (!symFile) err("Failed to open sym file \"%s\"", symFileName); + Defer closeSymFile{[&] { fclose(symFile); }}; fputs("; File generated by rgblink\n", symFile); @@ -542,8 +546,6 @@ static void writeSym() { for (uint32_t bank = 0; bank < sections[type].size(); bank++) writeSymBank(sections[type][bank], type, bank); } - - fclose(symFile); } // Writes the map file, if applicable. @@ -559,6 +561,7 @@ static void writeMap() { } if (!mapFile) err("Failed to open map file \"%s\"", mapFileName); + Defer closeMapFile{[&] { fclose(mapFile); }}; writeMapSummary(); @@ -568,8 +571,6 @@ static void writeMap() { for (uint32_t bank = 0; bank < sections[type].size(); bank++) writeMapBank(sections[type][bank], type, bank); } - - fclose(mapFile); } void out_WriteFiles() { diff --git a/src/link/script.y b/src/link/script.y index 1e0e3072..1eb87f50 100644 --- a/src/link/script.y +++ b/src/link/script.y @@ -225,7 +225,6 @@ static uint8_t parseHexDigit(int c) { } yy::parser::symbol_type yylex() { -try_again: // Can't use a `do {} while(0)` loop, otherwise compilers (wrongly) think it can end. auto &context = lexerStack.back(); auto c = context.file.sbumpc(); @@ -245,7 +244,7 @@ try_again: // Can't use a `do {} while(0)` loop, otherwise compilers (wrongly) t // Basically yywrap(). if (lexerStack.size() != 1) { lexerStack.pop_back(); - goto try_again; + return yylex(); } else if (!atEof) { // Inject a newline at EOF, to avoid errors for files that don't end with one. atEof = true; @@ -353,7 +352,7 @@ try_again: // Can't use a `do {} while(0)` loop, otherwise compilers (wrongly) t } scriptError(context, "Unknown keyword \"%s\"", ident.c_str()); - goto try_again; // Try lexing another token. + return yylex(); } else { scriptError(context, "Unexpected character '%s'", printChar(c)); // Keep reading characters until the EOL, to avoid reporting too many errors. @@ -363,7 +362,7 @@ try_again: // Can't use a `do {} while(0)` loop, otherwise compilers (wrongly) t } context.file.sbumpc(); } - goto try_again; + return yylex(); } // Not marking as unreachable; this will generate a warning if any codepath forgets to return. } diff --git a/src/link/sdas_obj.cpp b/src/link/sdas_obj.cpp index 05395d59..d7206b41 100644 --- a/src/link/sdas_obj.cpp +++ b/src/link/sdas_obj.cpp @@ -797,6 +797,10 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector } } +#undef expectEol +#undef expectToken +#undef getToken + if (!data.empty()) warning(&where, lineNo, "Last 'T' line had no 'R' line (ignored)"); if (fileSections.size() < expectedNbAreas) @@ -841,10 +845,4 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector // Calling `sect_AddSection` invalidates the contents of `fileSections`! sect_AddSection(std::move(section)); } - -#undef expectEol -#undef expectToken -#undef getToken - - fclose(file); }