diff --git a/.clang-format b/.clang-format index a11bfd09b..30cf41020 100644 --- a/.clang-format +++ b/.clang-format @@ -61,6 +61,7 @@ IndentPPDirectives: BeforeHash IndentRequires: true IndentWidth: 4 IndentWrappedFunctionNames: true +InsertBraces: true InsertNewlineAtEOF: true # Only support for Javascript as of clang-format 14... # InsertTrailingCommas: Wrapped diff --git a/src/asm/charmap.cpp b/src/asm/charmap.cpp index 36abaf1b7..b5875ee1d 100644 --- a/src/asm/charmap.cpp +++ b/src/asm/charmap.cpp @@ -52,16 +52,19 @@ bool charmap_ForEach( auto [nodeIdx, mapping] = std::move(prefixes.top()); prefixes.pop(); CharmapNode const &node = charmap.nodes[nodeIdx]; - if (node.isTerminal()) + if (node.isTerminal()) { mappings[nodeIdx] = mapping; + } for (unsigned c = 0; c < 256; c++) { - if (size_t nextIdx = node.next[c]; nextIdx) + if (size_t nextIdx = node.next[c]; nextIdx) { prefixes.push({nextIdx, mapping + static_cast(c)}); + } } } mapFunc(charmap.name); - for (auto [nodeIdx, mapping] : mappings) + for (auto [nodeIdx, mapping] : mappings) { charFunc(mapping, charmap.nodes[nodeIdx].value); + } } return !charmapList.empty(); } @@ -70,10 +73,11 @@ void charmap_New(std::string const &name, std::string const *baseName) { size_t baseIdx = SIZE_MAX; if (baseName != nullptr) { - if (auto search = charmapMap.find(*baseName); search == charmapMap.end()) + if (auto search = charmapMap.find(*baseName); search == charmapMap.end()) { error("Base charmap '%s' doesn't exist\n", baseName->c_str()); - else + } else { baseIdx = search->second; + } } if (charmapMap.find(name) != charmapMap.end()) { @@ -85,10 +89,11 @@ void charmap_New(std::string const &name, std::string const *baseName) { charmapMap[name] = charmapList.size(); Charmap &charmap = charmapList.emplace_back(); - if (baseIdx != SIZE_MAX) + if (baseIdx != SIZE_MAX) { charmap.nodes = charmapList[baseIdx].nodes; // Copies `charmapList[baseIdx].nodes` - else + } else { charmap.nodes.emplace_back(); // Zero-init the root node + } charmap.name = name; @@ -96,10 +101,11 @@ void charmap_New(std::string const &name, std::string const *baseName) { } void charmap_Set(std::string const &name) { - if (auto search = charmapMap.find(name); search == charmapMap.end()) + if (auto search = charmapMap.find(name); search == charmapMap.end()) { error("Charmap '%s' doesn't exist\n", name.c_str()); - else + } else { currentCharmap = &charmapList[search->second]; + } } void charmap_Push() { @@ -149,8 +155,9 @@ void charmap_Add(std::string const &mapping, std::vector &&value) { CharmapNode &node = charmap.nodes[nodeIdx]; - if (node.isTerminal()) + if (node.isTerminal()) { warning(WARNING_CHARMAP_REDEF, "Overriding charmap mapping\n"); + } std::swap(node.value, value); } @@ -162,8 +169,9 @@ bool charmap_HasChar(std::string const &input) { for (char c : input) { nodeIdx = charmap.nodes[nodeIdx].next[static_cast(c)]; - if (!nodeIdx) + if (!nodeIdx) { return false; + } } return charmap.nodes[nodeIdx].isTerminal(); @@ -188,8 +196,9 @@ size_t charmap_ConvertNext(std::string_view &input, std::vector *output for (size_t nodeIdx = 0; inputIdx < input.length();) { nodeIdx = charmap.nodes[nodeIdx].next[static_cast(input[inputIdx])]; - if (!nodeIdx) + if (!nodeIdx) { break; + } inputIdx++; // Consume that char @@ -209,8 +218,9 @@ size_t charmap_ConvertNext(std::string_view &input, std::vector *output if (matchIdx) { // A match was found, use it std::vector const &value = charmap.nodes[matchIdx].value; - if (output) + if (output) { output->insert(output->end(), RANGE(value)); + } matchLen = value.size(); } else if (inputIdx < input.length()) { // No match found, but there is some input left @@ -218,18 +228,20 @@ size_t charmap_ConvertNext(std::string_view &input, std::vector *output // This will write the codepoint's value to `output`, little-endian size_t codepointLen = readUTF8Char(output, input.data() + inputIdx); - if (codepointLen == 0) + if (codepointLen == 0) { error("Input string is not valid UTF-8\n"); + } // Warn if this character is not mapped but any others are - if (charmap.nodes.size() > 1) + if (charmap.nodes.size() > 1) { warning(WARNING_UNMAPPED_CHAR_1, "Unmapped character %s\n", printChar(firstChar)); - else if (charmap.name != DEFAULT_CHARMAP_NAME) + } else if (charmap.name != DEFAULT_CHARMAP_NAME) { warning( WARNING_UNMAPPED_CHAR_2, "Unmapped character %s not in " DEFAULT_CHARMAP_NAME " charmap\n", printChar(firstChar) ); + } inputIdx += codepointLen; matchLen = codepointLen; diff --git a/src/asm/fixpoint.cpp b/src/asm/fixpoint.cpp index 456db843a..d34a03a8d 100644 --- a/src/asm/fixpoint.cpp +++ b/src/asm/fixpoint.cpp @@ -25,10 +25,12 @@ static double fix2double(int32_t i, int32_t q) { } static int32_t double2fix(double d, int32_t q) { - if (isnan(d)) + if (isnan(d)) { return 0; - if (isinf(d)) + } + if (isinf(d)) { return d < 0 ? INT32_MIN : INT32_MAX; + } return static_cast(round(d * pow(2.0, q))); } @@ -75,8 +77,9 @@ int32_t fix_Mul(int32_t i, int32_t j, int32_t q) { int32_t fix_Div(int32_t i, int32_t j, int32_t q) { double dividend = fix2double(i, q); double divisor = fix2double(j, q); - if (fpclassify(divisor) == FP_ZERO) + if (fpclassify(divisor) == FP_ZERO) { return dividend < 0 ? INT32_MIN : dividend > 0 ? INT32_MAX : 0; + } return double2fix(dividend / divisor, q); } @@ -90,8 +93,9 @@ int32_t fix_Pow(int32_t i, int32_t j, int32_t q) { int32_t fix_Log(int32_t i, int32_t j, int32_t q) { double divisor = log(fix2double(j, q)); - if (fpclassify(divisor) == FP_ZERO) + if (fpclassify(divisor) == FP_ZERO) { return INT32_MAX; + } return double2fix(log(fix2double(i, q)) / divisor, q); } diff --git a/src/asm/format.cpp b/src/asm/format.cpp index 395f91b48..f9ba9fe3f 100644 --- a/src/asm/format.cpp +++ b/src/asm/format.cpp @@ -13,39 +13,44 @@ #include "asm/warning.hpp" void FormatSpec::useCharacter(int c) { - if (state == FORMAT_INVALID) + if (state == FORMAT_INVALID) { return; + } switch (c) { // sign case ' ': case '+': - if (state > FORMAT_SIGN) + if (state > FORMAT_SIGN) { goto invalid; + } state = FORMAT_EXACT; sign = c; break; // exact case '#': - if (state > FORMAT_EXACT) + if (state > FORMAT_EXACT) { goto invalid; + } state = FORMAT_ALIGN; exact = true; break; // align case '-': - if (state > FORMAT_ALIGN) + if (state > FORMAT_ALIGN) { goto invalid; + } state = FORMAT_WIDTH; alignLeft = true; break; // pad, width, and prec values case '0': - if (state < FORMAT_WIDTH) + if (state < FORMAT_WIDTH) { padZero = true; + } [[fallthrough]]; case '1': case '2': @@ -72,16 +77,18 @@ void FormatSpec::useCharacter(int c) { // width case '.': - if (state > FORMAT_WIDTH) + if (state > FORMAT_WIDTH) { goto invalid; + } state = FORMAT_FRAC; hasFrac = true; break; // prec case 'q': - if (state > FORMAT_PREC) + if (state > FORMAT_PREC) { goto invalid; + } state = FORMAT_PREC; hasPrec = true; break; @@ -95,8 +102,9 @@ void FormatSpec::useCharacter(int c) { case 'o': case 'f': case 's': - if (state >= FORMAT_DONE) + if (state >= FORMAT_DONE) { goto invalid; + } state = FORMAT_DONE; valid = true; type = c; @@ -110,8 +118,9 @@ void FormatSpec::useCharacter(int c) { } void FormatSpec::finishCharacters() { - if (!isValid()) + if (!isValid()) { state = FORMAT_INVALID; + } } static std::string escapeString(std::string const &str) { @@ -151,16 +160,21 @@ void FormatSpec::appendString(std::string &str, std::string const &value) const useType = 's'; } - if (sign) + if (sign) { error("Formatting string with sign flag '%c'\n", sign); - if (padZero) + } + if (padZero) { error("Formatting string with padding flag '0'\n"); - if (hasFrac) + } + if (hasFrac) { error("Formatting string with fractional width\n"); - if (hasPrec) + } + if (hasPrec) { error("Formatting string with fractional precision\n"); - if (useType != 's') + } + if (useType != 's') { error("Formatting string as type '%c'\n", useType); + } std::string useValue = exact ? escapeString(value) : value; size_t valueLen = useValue.length(); @@ -187,22 +201,27 @@ void FormatSpec::appendNumber(std::string &str, uint32_t value) const { } if (useType != 'X' && useType != 'x' && useType != 'b' && useType != 'o' && useType != 'f' - && useExact) + && useExact) { error("Formatting type '%c' with exact flag '#'\n", useType); - if (useType != 'f' && hasFrac) + } + if (useType != 'f' && hasFrac) { error("Formatting type '%c' with fractional width\n", useType); - if (useType != 'f' && hasPrec) + } + if (useType != 'f' && hasPrec) { error("Formatting type '%c' with fractional precision\n", useType); - if (useType == 's') + } + if (useType == 's') { error("Formatting number as type 's'\n"); + } char signChar = sign; // 0 or ' ' or '+' if (useType == 'd' || useType == 'f') { if (int32_t v = value; v < 0) { signChar = '-'; - if (v != INT32_MIN) + if (v != INT32_MIN) { value = -v; + } } } @@ -250,10 +269,11 @@ void FormatSpec::appendNumber(std::string &str, uint32_t value) const { } double fval = fabs(value / pow(2.0, usePrec)); - if (int fracWidthArg = static_cast(useFracWidth); useExact) + if (int fracWidthArg = static_cast(useFracWidth); useExact) { snprintf(valueBuf, sizeof(valueBuf), "%.*fq%zu", fracWidthArg, fval, usePrec); - else + } else { snprintf(valueBuf, sizeof(valueBuf), "%.*f", fracWidthArg, fval); + } } else if (useType == 'd') { // Decimal numbers may be formatted with a '-' sign by `snprintf`, so `abs` prevents that, // with a special case for `INT32_MIN` since `labs(INT32_MIN)` is UB. The sign will be @@ -278,27 +298,33 @@ void FormatSpec::appendNumber(std::string &str, uint32_t value) const { str.reserve(str.length() + totalLen); if (alignLeft) { - if (signChar) + if (signChar) { str += signChar; - if (prefixChar) + } + if (prefixChar) { str += prefixChar; + } str.append(valueBuf); str.append(padLen, ' '); } else { if (padZero) { // sign, then prefix, then zero padding - if (signChar) + if (signChar) { str += signChar; - if (prefixChar) + } + if (prefixChar) { str += prefixChar; + } str.append(padLen, '0'); } else { // space padding, then sign, then prefix str.append(padLen, ' '); - if (signChar) + if (signChar) { str += signChar; - if (prefixChar) + } + if (prefixChar) { str += prefixChar; + } } str.append(valueBuf); } diff --git a/src/asm/fstack.cpp b/src/asm/fstack.cpp index ed5ae5744..87bb85896 100644 --- a/src/asm/fstack.cpp +++ b/src/asm/fstack.cpp @@ -90,8 +90,9 @@ std::shared_ptr fstk_GetUniqueIDStr() { std::shared_ptr &str = contextStack.top().uniqueIDStr; // If a unique ID is allowed but has not been generated yet, generate one now. - if (str && str->empty()) + if (str && str->empty()) { *str = "_u"s + std::to_string(nextUniqueID++); + } return str; } @@ -103,27 +104,32 @@ MacroArgs *fstk_GetCurrentMacroArgs() { } void fstk_AddIncludePath(std::string const &path) { - if (path.empty()) + if (path.empty()) { return; + } std::string &includePath = includePaths.emplace_back(path); - if (includePath.back() != '/') + if (includePath.back() != '/') { includePath += '/'; + } } void fstk_SetPreIncludeFile(std::string const &path) { - if (!preIncludeName.empty()) + if (!preIncludeName.empty()) { warnx("Overriding pre-included filename %s", preIncludeName.c_str()); + } preIncludeName = path; - if (verbose) + if (verbose) { printf("Pre-included filename %s\n", preIncludeName.c_str()); + } } static void printDep(std::string const &path) { if (dependFile) { fprintf(dependFile, "%s: %s\n", targetFileName.c_str(), path.c_str()); - if (generatePhonyDeps) + if (generatePhonyDeps) { fprintf(dependFile, "%s:\n", path.c_str()); + } } } @@ -141,20 +147,22 @@ std::optional fstk_FindFile(std::string const &path) { } errno = ENOENT; - if (generatedMissingIncludes) + if (generatedMissingIncludes) { printDep(path); + } return std::nullopt; } bool yywrap() { uint32_t ifDepth = lexer_GetIFDepth(); - if (ifDepth != 0) + if (ifDepth != 0) { fatalerror( "Ended block with %" PRIu32 " unterminated IF construct%s\n", ifDepth, ifDepth == 1 ? "" : "s" ); + } if (Context &context = contextStack.top(); context.fileInfo->type == NODE_REPT) { // The context is a REPT or FOR block, which may loop @@ -177,8 +185,9 @@ bool yywrap() { Symbol *sym = sym_AddVar(context.forName, context.forValue); // This error message will refer to the current iteration - if (sym->type != SYM_VAR) + if (sym->type != SYM_VAR) { fatalerror("Failed to update FOR symbol value\n"); + } } // Advance to the next iteration fileInfoIters.front()++; @@ -199,8 +208,9 @@ bool yywrap() { } static void checkRecursionDepth() { - if (contextStack.size() > maxRecursionDepth) + if (contextStack.size() > maxRecursionDepth) { fatalerror("Recursion limit (%zu) exceeded\n", maxRecursionDepth); + } } static bool newFileContext(std::string const &filePath, bool updateStateNow) { @@ -298,8 +308,9 @@ void fstk_RunInclude(std::string const &path, bool preInclude) { if (!fullPath) { if (generatedMissingIncludes && !preInclude) { - if (verbose) + if (verbose) { printf("Aborting (-MG) on INCLUDE file '%s' (%s)\n", path.c_str(), strerror(errno)); + } failedOnMissingInclude = true; } else { error("Unable to open included file '%s': %s\n", path.c_str(), strerror(errno)); @@ -307,18 +318,20 @@ void fstk_RunInclude(std::string const &path, bool preInclude) { return; } - if (!newFileContext(*fullPath, false)) + if (!newFileContext(*fullPath, false)) { fatalerror("Failed to set up lexer for file include\n"); + } } void fstk_RunMacro(std::string const ¯oName, std::shared_ptr macroArgs) { Symbol *macro = sym_FindExactSymbol(macroName); if (!macro) { - if (sym_IsPurgedExact(macroName)) + if (sym_IsPurgedExact(macroName)) { error("Macro \"%s\" not defined; it was purged\n", macroName.c_str()); - else + } else { error("Macro \"%s\" not defined\n", macroName.c_str()); + } return; } if (macro->type != SYM_MACRO) { @@ -330,8 +343,9 @@ void fstk_RunMacro(std::string const ¯oName, std::shared_ptr macr } void fstk_RunRept(uint32_t count, int32_t reptLineNo, ContentSpan const &span) { - if (count == 0) + if (count == 0) { return; + } newReptContext(reptLineNo, span, count); } @@ -344,24 +358,28 @@ void fstk_RunFor( int32_t reptLineNo, ContentSpan const &span ) { - if (Symbol *sym = sym_AddVar(symName, start); sym->type != SYM_VAR) + if (Symbol *sym = sym_AddVar(symName, start); sym->type != SYM_VAR) { return; + } uint32_t count = 0; - if (step > 0 && start < stop) + if (step > 0 && start < stop) { count = (static_cast(stop) - start - 1) / step + 1; - else if (step < 0 && stop < start) + } else if (step < 0 && stop < start) { count = (static_cast(start) - stop - 1) / -static_cast(step) + 1; - else if (step == 0) + } else if (step == 0) { error("FOR cannot have a step value of 0\n"); + } - if ((step > 0 && start > stop) || (step < 0 && start < stop)) + if ((step > 0 && start > stop) || (step < 0 && start < stop)) { warning( WARNING_BACKWARDS_FOR, "FOR goes backwards from %d to %d by %d\n", start, stop, step ); + } - if (count == 0) + if (count == 0) { return; + } Context &context = newReptContext(reptLineNo, span, count); context.isForLoop = true; @@ -385,17 +403,20 @@ bool fstk_Break() { } void fstk_NewRecursionDepth(size_t newDepth) { - if (contextStack.size() > newDepth + 1) + if (contextStack.size() > newDepth + 1) { fatalerror("Recursion limit (%zu) exceeded\n", newDepth); + } maxRecursionDepth = newDepth; } void fstk_Init(std::string const &mainPath, size_t maxDepth) { - if (!newFileContext(mainPath, true)) + if (!newFileContext(mainPath, true)) { fatalerror("Failed to open main file\n"); + } maxRecursionDepth = maxDepth; - if (!preIncludeName.empty()) + if (!preIncludeName.empty()) { fstk_RunInclude(preIncludeName, true); + } } diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index 04196875b..d27364bc9 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -81,8 +81,9 @@ static char *mapFile(int fd, std::string const &path, size_t size) { // The implementation may not support MAP_PRIVATE; try again with MAP_SHARED // instead, offering, I believe, weaker guarantees about external modifications to // the file while reading it. That's still better than not opening it at all, though. - if (verbose) + if (verbose) { printf("mmap(%s, MAP_PRIVATE) failed, retrying with MAP_SHARED\n", path.c_str()); + } mappingAddr = mmap(nullptr, size, PROT_READ, MAP_SHARED, fd, 0); } return mappingAddr != MAP_FAILED ? static_cast(mappingAddr) : nullptr; @@ -119,8 +120,9 @@ struct CaseInsensitive { size_t operator()(std::string const &str) const { size_t hash = 0x811C9DC5; - for (char const &c : str) + for (char const &c : str) { hash = (hash ^ toupper(c)) * 16777619; + } return hash; } @@ -375,8 +377,9 @@ void lexer_IncIFDepth() { } void lexer_DecIFDepth() { - if (lexerState->ifStack.empty()) + if (lexerState->ifStack.empty()) { fatalerror("Found ENDC outside of an IF construct\n"); + } lexerState->ifStack.pop_front(); } @@ -405,8 +408,9 @@ bool LexerState::setFileAsNextState(std::string const &filePath, bool updateStat if (filePath == "-") { path = ""; content.emplace(STDIN_FILENO); - if (verbose) + if (verbose) { printf("Opening stdin\n"); + } } else { struct stat statBuf; if (stat(filePath.c_str(), &statBuf) != 0) { @@ -430,8 +434,9 @@ bool LexerState::setFileAsNextState(std::string const &filePath, bool updateStat content.emplace( std::shared_ptr(mappingAddr, FileUnmapDeleter(size)), size ); - if (verbose) + if (verbose) { printf("File \"%s\" is mmap()ped\n", path.c_str()); + } isMmapped = true; } } @@ -452,10 +457,11 @@ bool LexerState::setFileAsNextState(std::string const &filePath, bool updateStat } clear(0); - if (updateStateNow) + if (updateStateNow) { lexerState = this; - else + } else { lexerStateEOL = this; + } return true; } @@ -501,8 +507,9 @@ BufferedContent::~BufferedContent() { void BufferedContent::advance() { assume(offset < std::size(buf)); offset++; - if (offset == std::size(buf)) + if (offset == std::size(buf)) { offset = 0; // Wrap around if necessary + } assume(size > 0); size--; } @@ -519,16 +526,19 @@ void BufferedContent::refill() { size_t nbReadChars = readMore(startIndex, nbExpectedChars); startIndex += nbReadChars; - if (startIndex == std::size(buf)) + if (startIndex == std::size(buf)) { startIndex = 0; + } // If the read was incomplete, don't perform a second read target -= nbReadChars; - if (nbReadChars < nbExpectedChars) + if (nbReadChars < nbExpectedChars) { target = 0; + } } - if (target != 0) + if (target != 0) { readMore(startIndex, target); + } } size_t BufferedContent::readMore(size_t startIndex, size_t nbChars) { @@ -536,8 +546,9 @@ size_t BufferedContent::readMore(size_t startIndex, size_t nbChars) { assume(startIndex + nbChars <= std::size(buf)); ssize_t nbReadChars = read(fd, &buf[startIndex], nbChars); - if (nbReadChars == -1) + if (nbReadChars == -1) { fatalerror("Error while reading \"%s\": %s\n", lexerState->path.c_str(), strerror(errno)); + } size += nbReadChars; @@ -556,19 +567,22 @@ void lexer_ToggleStringExpansion(bool enable) { // Functions for the actual lexer to obtain characters static void beginExpansion(std::shared_ptr str, std::optional name) { - if (name) + if (name) { lexer_CheckRecursionDepth(); + } // Do not expand empty strings - if (str->empty()) + if (str->empty()) { return; + } lexerState->expansions.push_front({.name = name, .contents = str, .offset = 0}); } void lexer_CheckRecursionDepth() { - if (lexerState->expansions.size() > maxRecursionDepth + 1) + if (lexerState->expansions.size() > maxRecursionDepth + 1) { fatalerror("Recursion limit (%zu) exceeded\n", maxRecursionDepth); + } } static bool isMacroChar(char c) { @@ -619,10 +633,11 @@ static uint32_t readBracketedMacroArgNum() { Symbol const *sym = sym_FindScopedValidSymbol(symName); if (!sym) { - if (sym_IsPurgedScoped(symName)) + if (sym_IsPurgedScoped(symName)) { error("Bracketed symbol \"%s\" does not exist; it was purged\n", symName.c_str()); - else + } else { error("Bracketed symbol \"%s\" does not exist\n", symName.c_str()); + } num = 0; symbolError = true; } else if (!sym->isNumeric()) { @@ -707,21 +722,25 @@ static std::shared_ptr readMacroArg(char name) { int LexerState::peekChar() { // This is `.peekCharAhead()` modified for zero lookahead distance for (Expansion &exp : expansions) { - if (exp.offset < exp.size()) + if (exp.offset < exp.size()) { return static_cast((*exp.contents)[exp.offset]); + } } if (content.holds()) { auto &view = content.get(); - if (view.offset < view.span.size) + if (view.offset < view.span.size) { return static_cast(view.span.ptr[view.offset]); + } } else { auto &cbuf = content.get(); - if (cbuf.size == 0) + if (cbuf.size == 0) { cbuf.refill(); + } assume(cbuf.offset < std::size(cbuf.buf)); - if (cbuf.size > 0) + if (cbuf.size > 0) { return static_cast(cbuf.buf[cbuf.offset]); + } } // If there aren't enough chars, give up @@ -736,22 +755,26 @@ int LexerState::peekCharAhead() { // An expansion that has reached its end will have `exp.offset` == `exp.size()`, // and `.peekCharAhead()` will continue with its parent assume(exp.offset <= exp.size()); - if (exp.offset + distance < exp.size()) + if (exp.offset + distance < exp.size()) { return static_cast((*exp.contents)[exp.offset + distance]); + } distance -= exp.size() - exp.offset; } if (content.holds()) { auto &view = content.get(); - if (view.offset + distance < view.span.size) + if (view.offset + distance < view.span.size) { return static_cast(view.span.ptr[view.offset + distance]); + } } else { auto &cbuf = content.get(); assume(distance < std::size(cbuf.buf)); - if (cbuf.size <= distance) + if (cbuf.size <= distance) { cbuf.refill(); - if (cbuf.size > distance) + } + if (cbuf.size > distance) { return static_cast(cbuf.buf[(cbuf.offset + distance) % std::size(cbuf.buf)]); + } } // If there aren't enough chars, give up @@ -765,8 +788,9 @@ static std::shared_ptr readInterpolation(size_t depth); static int peek() { int c = lexerState->peekChar(); - if (lexerState->macroArgScanDistance > 0) + if (lexerState->macroArgScanDistance > 0) { return c; + } lexerState->macroArgScanDistance++; // Do not consider again @@ -809,8 +833,9 @@ static int peek() { static void shiftChar() { if (lexerState->capturing) { - if (lexerState->captureBuf) + if (lexerState->captureBuf) { lexerState->captureBuf->push_back(peek()); + } lexerState->captureSize++; } @@ -840,14 +865,16 @@ static int nextChar() { int c = peek(); // If not at EOF, advance read position - if (c != EOF) + if (c != EOF) { shiftChar(); + } return c; } static void handleCRLF(int c) { - if (c == '\r' && peek() == '\n') + if (c == '\r' && peek() == '\n') { shiftChar(); + } } static auto scopedDisableExpansions() { @@ -870,13 +897,15 @@ uint32_t lexer_GetColNo() { } void lexer_DumpStringExpansions() { - if (!lexerState) + if (!lexerState) { return; + } for (Expansion &exp : lexerState->expansions) { // Only register EQUS expansions, not string args - if (exp.name) + if (exp.name) { fprintf(stderr, "while expanding symbol \"%s\"\n", exp.name->c_str()); + } } } @@ -896,8 +925,9 @@ static void discardBlockComment() { handleCRLF(c); [[fallthrough]]; case '\n': - if (lexerState->expansions.empty()) + if (lexerState->expansions.empty()) { nextLine(); + } continue; case '/': if (peek() == '*') { @@ -921,8 +951,9 @@ static void discardComment() { for (;; shiftChar()) { int c = peek(); - if (c == EOF || c == '\r' || c == '\n') + if (c == EOF || c == '\r' || c == '\n') { break; + } } } @@ -936,8 +967,9 @@ static void discardLineContinuation() { shiftChar(); // Handle CRLF before nextLine() since shiftChar updates colNo handleCRLF(c); - if (lexerState->expansions.empty()) + if (lexerState->expansions.empty()) { nextLine(); + } break; } else if (c == ';') { discardComment(); @@ -968,12 +1000,14 @@ static uint32_t readNumber(int radix, uint32_t baseValue) { for (;; shiftChar()) { int c = peek(); - if (c == '_') + if (c == '_') { continue; - else if (c < '0' || c > '0' + radix - 1) + } else if (c < '0' || c > '0' + radix - 1) { break; - if (value > (UINT32_MAX - (c - '0')) / radix) + } + if (value > (UINT32_MAX - (c - '0')) / radix) { warning(WARNING_LARGE_CONSTANT, "Integer constant is too large\n"); + } value = value * radix + (c - '0'); } @@ -1005,8 +1039,9 @@ static uint32_t readFractionalPart(uint32_t integer) { warning(WARNING_LARGE_CONSTANT, "Precision of fixed-point constant is too large\n"); // Discard any additional digits shiftChar(); - while (c = peek(), (c >= '0' && c <= '9') || c == '_') + while (c = peek(), (c >= '0' && c <= '9') || c == '_') { shiftChar(); + } break; } value = value * 10 + (c - '0'); @@ -1023,16 +1058,18 @@ static uint32_t readFractionalPart(uint32_t integer) { } if (precision == 0) { - if (state >= READFRACTIONALPART_PRECISION) + if (state >= READFRACTIONALPART_PRECISION) { error("Invalid fixed-point constant, no significant digits after 'q'\n"); + } precision = fixPrecision; } else if (precision > 31) { error("Fixed-point constant precision must be between 1 and 31\n"); precision = fixPrecision; } - if (integer >= (1ULL << (32 - precision))) + if (integer >= (1ULL << (32 - precision))) { warning(WARNING_LARGE_CONSTANT, "Magnitude of fixed-point constant is too large\n"); + } // Cast to unsigned avoids undefined overflow behavior uint32_t fractional = @@ -1051,16 +1088,18 @@ static uint32_t readBinaryNumber() { int bit; // Check for '_' after digits in case one of the digits is '_' - if (c == binDigits[0]) + if (c == binDigits[0]) { bit = 0; - else if (c == binDigits[1]) + } else if (c == binDigits[1]) { bit = 1; - else if (c == '_') + } else if (c == '_') { continue; - else + } else { break; - if (value > (UINT32_MAX - bit) / 2) + } + if (value > (UINT32_MAX - bit) / 2) { warning(WARNING_LARGE_CONSTANT, "Integer constant is too large\n"); + } value = value * 2 + bit; } @@ -1074,26 +1113,29 @@ static uint32_t readHexNumber() { for (;; shiftChar()) { int c = peek(); - if (c >= 'a' && c <= 'f') + if (c >= 'a' && c <= 'f') { c = c - 'a' + 10; - else if (c >= 'A' && c <= 'F') + } else if (c >= 'A' && c <= 'F') { c = c - 'A' + 10; - else if (c >= '0' && c <= '9') + } else if (c >= '0' && c <= '9') { c = c - '0'; - else if (c == '_' && !empty) + } else if (c == '_' && !empty) { continue; - else + } else { break; + } - if (value > (UINT32_MAX - c) / 16) + if (value > (UINT32_MAX - c) / 16) { warning(WARNING_LARGE_CONSTANT, "Integer constant is too large\n"); + } value = value * 16 + c; empty = false; } - if (empty) + if (empty) { error("Invalid integer constant, no digits after '$'\n"); + } return value; } @@ -1109,34 +1151,37 @@ static uint32_t readGfxConstant() { uint32_t pixel; // Check for '_' after digits in case one of the digits is '_' - if (c == gfxDigits[0]) + if (c == gfxDigits[0]) { pixel = 0; - else if (c == gfxDigits[1]) + } else if (c == gfxDigits[1]) { pixel = 1; - else if (c == gfxDigits[2]) + } else if (c == gfxDigits[2]) { pixel = 2; - else if (c == gfxDigits[3]) + } else if (c == gfxDigits[3]) { pixel = 3; - else if (c == '_' && width > 0) + } else if (c == '_' && width > 0) { continue; - else + } else { break; + } if (width < 8) { bitPlaneLower = bitPlaneLower << 1 | (pixel & 1); bitPlaneUpper = bitPlaneUpper << 1 | (pixel >> 1); } - if (width < 9) + if (width < 9) { width++; + } } - if (width == 0) + if (width == 0) { error("Invalid graphics constant, no digits after '`'\n"); - else if (width == 9) + } else if (width == 9) { warning( WARNING_LARGE_CONSTANT, "Graphics constant is too long, only first 8 pixels considered\n" ); + } return bitPlaneUpper << 8 | bitPlaneLower; } @@ -1164,8 +1209,9 @@ static Token readIdentifier(char firstChar, bool raw) { identifier += c; // If the char was a dot, mark the identifier as local - if (c == '.') + if (c == '.') { tokenType = T_(LOCAL_ID); + } } // Attempt to check for a keyword if the identifier is not raw @@ -1179,8 +1225,9 @@ static Token readIdentifier(char firstChar, bool raw) { } // Label scopes `.` and `..` are the only nonlocal identifiers that start with a dot - if (identifier.find_first_not_of('.') == identifier.npos) + if (identifier.find_first_not_of('.') == identifier.npos) { tokenType = T_(ID); + } return Token(tokenType, identifier); } @@ -1188,8 +1235,9 @@ static Token readIdentifier(char firstChar, bool raw) { // Functions to read strings static std::shared_ptr readInterpolation(size_t depth) { - if (depth > maxRecursionDepth) + if (depth > maxRecursionDepth) { fatalerror("Recursion limit (%zu) exceeded\n", maxRecursionDepth); + } std::string fmtBuf; FormatSpec fmt{}; @@ -1217,11 +1265,13 @@ static std::shared_ptr readInterpolation(size_t depth) { break; } else if (c == ':' && !fmt.isFinished()) { // Format spec, only once shiftChar(); - for (char f : fmtBuf) + for (char f : fmtBuf) { fmt.useCharacter(f); + } fmt.finishCharacters(); - if (!fmt.isValid()) + if (!fmt.isValid()) { error("Invalid format spec '%s'\n", fmtBuf.c_str()); + } fmtBuf.clear(); // Now that format has been set, restart at beginning of string } else { shiftChar(); @@ -1248,10 +1298,11 @@ static std::shared_ptr readInterpolation(size_t depth) { Symbol const *sym = sym_FindScopedValidSymbol(fmtBuf); if (!sym || !sym->isDefined()) { - if (sym_IsPurgedScoped(fmtBuf)) + if (sym_IsPurgedScoped(fmtBuf)) { error("Interpolated symbol \"%s\" does not exist; it was purged\n", fmtBuf.c_str()); - else + } else { error("Interpolated symbol \"%s\" does not exist\n", fmtBuf.c_str()); + } } else if (sym->type == SYM_EQUS) { auto buf = std::make_shared(); fmt.appendString(*buf, *sym->getEqus()); @@ -1335,8 +1386,9 @@ static std::string readString(bool raw) { case '"': if (multiline) { // Only """ ends a multi-line string - if (peek() != '"') + if (peek() != '"') { break; + } shiftChar(); if (peek() != '"') { str += '"'; @@ -1347,8 +1399,9 @@ static std::string readString(bool raw) { return str; case '\\': // Character escape or macro arg - if (raw) + if (raw) { break; + } c = peek(); switch (c) { case '\\': @@ -1415,8 +1468,9 @@ static std::string readString(bool raw) { break; case '{': // Symbol interpolation - if (raw) + if (raw) { break; + } // We'll be exiting the string scope, so re-enable expansions // (Not interpolations, since they're handled by the function itself...) lexerState->disableMacroArgs = false; @@ -1477,12 +1531,14 @@ static void appendStringLiteral(std::string &str, bool raw) { case '"': if (multiline) { // Only """ ends a multi-line string - if (peek() != '"') + if (peek() != '"') { break; + } str += '"'; shiftChar(); - if (peek() != '"') + if (peek() != '"') { break; + } str += '"'; shiftChar(); } @@ -1490,8 +1546,9 @@ static void appendStringLiteral(std::string &str, bool raw) { return; case '\\': // Character escape or macro arg - if (raw) + if (raw) { break; + } c = peek(); switch (c) { // Character escape @@ -1549,8 +1606,9 @@ static void appendStringLiteral(std::string &str, bool raw) { break; case '{': // Symbol interpolation - if (raw) + if (raw) { break; + } // We'll be exiting the string scope, so re-enable expansions // (Not interpolations, since they're handled by the function itself...) lexerState->disableMacroArgs = false; @@ -1849,12 +1907,14 @@ static Token yylex_NORMAL() { // An ELIF after a taken IF needs to not evaluate its condition if (token.type == T_(POP_ELIF) && lexerState->lastToken == T_(NEWLINE) - && lexer_GetIFDepth() > 0 && lexer_RanIFBlock() && !lexer_ReachedELSEBlock()) + && lexer_GetIFDepth() > 0 && lexer_RanIFBlock() && !lexer_ReachedELSEBlock()) { return yylex_SKIP_TO_ENDC(); + } // If a keyword, don't try to expand - if (token.type != T_(ID) && token.type != T_(LOCAL_ID)) + if (token.type != T_(ID) && token.type != T_(LOCAL_ID)) { return token; + } // `token` is either an `ID` or a `LOCAL_ID`, and both have a `std::string` value. assume(token.value.holds()); @@ -1885,8 +1945,9 @@ static Token yylex_NORMAL() { // character *immediately* follows the identifier. Thus, at the beginning of a line, // "Label:" and "mac:" are treated as label definitions, but "Label :" and "mac :" // are treated as macro invocations. - if (token.type == T_(ID) && peek() == ':') + if (token.type == T_(ID) && peek() == ':') { token.type = T_(LABEL); + } return token; } @@ -1916,8 +1977,9 @@ static Token yylex_RAW() { shiftChar(); c = peek(); // If not a line continuation, handle as a normal char - if (!isWhitespace(c) && c != '\n' && c != '\r') + if (!isWhitespace(c) && c != '\n' && c != '\r') { goto backslash; + } // Line continuations count as "whitespace" discardLineContinuation(); } else { @@ -1963,18 +2025,21 @@ static Token yylex_RAW() { break; case ',': // End of macro arg - if (parenDepth == 0) + if (parenDepth == 0) { goto finish; + } goto append; case '(': // Open parentheses inside macro args - if (parenDepth < UINT_MAX) + if (parenDepth < UINT_MAX) { parenDepth++; + } goto append; case ')': // Close parentheses inside macro args - if (parenDepth > 0) + if (parenDepth > 0) { parenDepth--; + } goto append; case '\\': // Character escape @@ -2054,8 +2119,9 @@ static Token yylex_RAW() { // an empty raw string before it). This will not be treated as a // macro argument. To pass an empty last argument, use a second // trailing comma. - if (!str.empty()) + if (!str.empty()) { return Token(T_(STRING), str); + } lexer_SetMode(LEXER_NORMAL); if (c == '\r' || c == '\n') { @@ -2087,8 +2153,9 @@ static Token skipIfBlock(bool toEndc) { for (;; shiftChar()) { c = peek(); - if (!isWhitespace(c)) + if (!isWhitespace(c)) { break; + } } if (startsIdentifier(c)) { @@ -2099,23 +2166,28 @@ static Token skipIfBlock(bool toEndc) { break; case T_(POP_ELIF): - if (lexer_ReachedELSEBlock()) + if (lexer_ReachedELSEBlock()) { fatalerror("Found ELIF after an ELSE block\n"); - if (!toEndc && lexer_GetIFDepth() == startingDepth) + } + if (!toEndc && lexer_GetIFDepth() == startingDepth) { return token; + } break; case T_(POP_ELSE): - if (lexer_ReachedELSEBlock()) + if (lexer_ReachedELSEBlock()) { fatalerror("Found ELSE after an ELSE block\n"); + } lexer_ReachELSEBlock(); - if (!toEndc && lexer_GetIFDepth() == startingDepth) + if (!toEndc && lexer_GetIFDepth() == startingDepth) { return token; + } break; case T_(POP_ENDC): - if (lexer_GetIFDepth() == startingDepth) + if (lexer_GetIFDepth() == startingDepth) { return token; + } lexer_DecIFDepth(); break; @@ -2172,8 +2244,9 @@ static Token yylex_SKIP_TO_ENDR() { for (;;) { c = peek(); - if (!isWhitespace(c)) + if (!isWhitespace(c)) { break; + } shiftChar(); } @@ -2187,8 +2260,9 @@ static Token yylex_SKIP_TO_ENDR() { case T_(POP_ENDR): depth--; - if (!depth) + if (!depth) { return Token(T_(YYEOF)); // yywrap() will finish the REPT/FOR loop + } break; case T_(POP_IF): @@ -2234,11 +2308,13 @@ yy::parser::symbol_type yylex() { lexerState = lexerStateEOL; lexerStateEOL = nullptr; } - if (lexerState->lastToken == T_(EOB) && yywrap()) + if (lexerState->lastToken == T_(EOB) && yywrap()) { return yy::parser::make_YYEOF(); + } // Newlines read within an expansion should not increase the line count - if (lexerState->atLineStart && lexerState->expansions.empty()) + if (lexerState->atLineStart && lexerState->expansions.empty()) { nextLine(); + } static Token (* const lexerModeFuncs[NB_LEXER_MODES])() = { yylex_NORMAL, @@ -2299,8 +2375,9 @@ static Capture startCapture() { static void endCapture(Capture &capture) { // This being `nullptr` means we're capturing from the capture buffer, which is reallocated // during the whole capture process, and so MUST be retrieved at the end - if (!capture.span.ptr) + if (!capture.span.ptr) { capture.span.ptr = lexerState->makeSharedCaptureBufPtr(); + } capture.span.size = lexerState->captureSize; // ENDR/ENDM or EOF puts us past the start of the line diff --git a/src/asm/macro.cpp b/src/asm/macro.cpp index 6f6a4305c..dc5f3a465 100644 --- a/src/asm/macro.cpp +++ b/src/asm/macro.cpp @@ -17,13 +17,15 @@ std::shared_ptr MacroArgs::getArg(uint32_t i) const { std::shared_ptr MacroArgs::getAllArgs() const { size_t nbArgs = args.size(); - if (shift >= nbArgs) + if (shift >= nbArgs) { return std::make_shared(""); + } size_t len = 0; - for (uint32_t i = shift; i < nbArgs; i++) + for (uint32_t i = shift; i < nbArgs; i++) { len += args[i]->length() + 1; // 1 for comma + } auto str = std::make_shared(); str->reserve(len + 1); // 1 for comma @@ -34,16 +36,18 @@ std::shared_ptr MacroArgs::getAllArgs() const { str->append(*arg); // Commas go between args and after a last empty arg - if (i < nbArgs - 1 || arg->empty()) + if (i < nbArgs - 1 || arg->empty()) { str->push_back(','); // no space after comma + } } return str; } void MacroArgs::appendArg(std::shared_ptr arg) { - if (arg->empty()) + if (arg->empty()) { warning(WARNING_EMPTY_MACRO_ARG, "Empty macro argument\n"); + } args.push_back(arg); } diff --git a/src/asm/main.cpp b/src/asm/main.cpp index 950987ef6..2da5e28a5 100644 --- a/src/asm/main.cpp +++ b/src/asm/main.cpp @@ -37,8 +37,9 @@ static std::string make_escape(std::string &str) { for (;;) { // All dollars needs to be doubled size_t nextPos = str.find("$", pos); - if (nextPos == std::string::npos) + if (nextPos == std::string::npos) { break; + } escaped.append(str, pos, nextPos - pos); escaped.append("$$"); pos = nextPos + literal_strlen("$"); @@ -111,12 +112,14 @@ int main(int argc, char *argv[]) { time_t now = time(nullptr); // Support SOURCE_DATE_EPOCH for reproducible builds // https://reproducible-builds.org/docs/source-date-epoch/ - if (char const *sourceDateEpoch = getenv("SOURCE_DATE_EPOCH"); sourceDateEpoch) + if (char const *sourceDateEpoch = getenv("SOURCE_DATE_EPOCH"); sourceDateEpoch) { now = static_cast(strtoul(sourceDateEpoch, nullptr, 0)); + } Defer closeDependFile{[&] { - if (dependFile) + if (dependFile) { fclose(dependFile); + } }}; // Perform some init for below @@ -133,18 +136,20 @@ int main(int argc, char *argv[]) { std::unordered_map> stateFileSpecs; std::string newTarget; // Maximum of 100 errors only applies if rgbasm is printing errors to a terminal. - if (isatty(STDERR_FILENO)) + if (isatty(STDERR_FILENO)) { maxErrors = 100; + } for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, nullptr)) != -1;) { switch (ch) { char *endptr; case 'b': - if (strlen(musl_optarg) == 2) + if (strlen(musl_optarg) == 2) { opt_B(musl_optarg); - else + } else { errx("Must specify exactly 2 characters for option 'b'"); + } break; char *equals; @@ -163,10 +168,11 @@ int main(int argc, char *argv[]) { break; case 'g': - if (strlen(musl_optarg) == 4) + if (strlen(musl_optarg) == 4) { opt_G(musl_optarg); - else + } else { errx("Must specify exactly 4 characters for option 'g'"); + } break; case 'h': @@ -178,8 +184,9 @@ int main(int argc, char *argv[]) { break; case 'M': - if (dependFile) + if (dependFile) { warnx("Overriding dependfile %s", dependFileName); + } if (strcmp("-", musl_optarg)) { dependFile = fopen(musl_optarg, "w"); dependFileName = musl_optarg; @@ -187,8 +194,9 @@ int main(int argc, char *argv[]) { dependFile = stdout; dependFileName = ""; } - if (dependFile == nullptr) + if (dependFile == nullptr) { err("Failed to open dependfile \"%s\"", dependFileName); + } break; case 'o': @@ -203,11 +211,13 @@ int main(int argc, char *argv[]) { case 'p': padByte = strtoul(musl_optarg, &endptr, 0); - if (musl_optarg[0] == '\0' || *endptr != '\0') + if (musl_optarg[0] == '\0' || *endptr != '\0') { errx("Invalid argument for option 'p'"); + } - if (padByte > 0xFF) + if (padByte > 0xFF) { errx("Argument for option 'p' must be between 0 and 0xFF"); + } opt_P(padByte); break; @@ -216,15 +226,18 @@ int main(int argc, char *argv[]) { char const *precisionArg; case 'Q': precisionArg = musl_optarg; - if (precisionArg[0] == '.') + if (precisionArg[0] == '.') { precisionArg++; + } precision = strtoul(precisionArg, &endptr, 0); - if (musl_optarg[0] == '\0' || *endptr != '\0') + if (musl_optarg[0] == '\0' || *endptr != '\0') { errx("Invalid argument for option 'Q'"); + } - if (precision < 1 || precision > 31) + if (precision < 1 || precision > 31) { errx("Argument for option 'Q' must be between 1 and 31"); + } opt_Q(precision); break; @@ -232,35 +245,41 @@ int main(int argc, char *argv[]) { case 'r': maxDepth = strtoul(musl_optarg, &endptr, 0); - if (musl_optarg[0] == '\0' || *endptr != '\0') + if (musl_optarg[0] == '\0' || *endptr != '\0') { errx("Invalid argument for option 'r'"); + } break; case 's': { // Split ":" so `musl_optarg` is "" and `name` is "" char *name = strchr(musl_optarg, ':'); - if (!name) + if (!name) { errx("Invalid argument for option 's'"); + } *name++ = '\0'; std::vector features; for (char *feature = musl_optarg; feature;) { // Split "," so `feature` is "" and `next` is "" char *next = strchr(feature, ','); - if (next) + if (next) { *next++ = '\0'; + } // Trim whitespace from the beginning of `feature`... feature += strspn(feature, " \t"); // ...and from the end - if (char *end = strpbrk(feature, " \t"); end) + if (char *end = strpbrk(feature, " \t"); end) { *end = '\0'; + } // A feature must be specified - if (*feature == '\0') + if (*feature == '\0') { errx("Empty feature for option 's'"); + } // Parse the `feature` and update the `features` list if (!strcasecmp(feature, "all")) { - if (!features.empty()) + if (!features.empty()) { warnx("Redundant feature before \"%s\" for option 's'", feature); + } features.assign({STATE_EQU, STATE_VAR, STATE_EQUS, STATE_CHAR, STATE_MACRO}); } else { StateFeature value = !strcasecmp(feature, "equ") ? STATE_EQU @@ -280,10 +299,12 @@ int main(int argc, char *argv[]) { feature = next; } - if (stateFileSpecs.find(name) != stateFileSpecs.end()) + if (stateFileSpecs.find(name) != stateFileSpecs.end()) { warnx("Overriding state filename %s", name); - if (verbose) + } + if (verbose) { printf("State filename %s\n", name); + } stateFileSpecs.emplace(name, std::move(features)); break; } @@ -308,11 +329,13 @@ int main(int argc, char *argv[]) { case 'X': maxValue = strtoul(musl_optarg, &endptr, 0); - if (musl_optarg[0] == '\0' || *endptr != '\0') + if (musl_optarg[0] == '\0' || *endptr != '\0') { errx("Invalid argument for option 'X'"); + } - if (maxValue > UINT_MAX) + if (maxValue > UINT_MAX) { errx("Argument for option 'X' must be between 0 and %u", UINT_MAX); + } maxErrors = maxValue; break; @@ -331,10 +354,12 @@ int main(int argc, char *argv[]) { case 'Q': case 'T': newTarget = musl_optarg; - if (depType == 'Q') + if (depType == 'Q') { newTarget = make_escape(newTarget); - if (!targetFileName.empty()) + } + if (!targetFileName.empty()) { targetFileName += ' '; + } targetFileName += newTarget; break; } @@ -347,8 +372,9 @@ int main(int argc, char *argv[]) { } } - if (targetFileName.empty() && !objectFileName.empty()) + if (targetFileName.empty() && !objectFileName.empty()) { targetFileName = objectFileName; + } if (argc == musl_optind) { fputs( @@ -364,13 +390,15 @@ int main(int argc, char *argv[]) { std::string mainFileName = argv[musl_optind]; - if (verbose) + if (verbose) { printf("Assembling %s\n", mainFileName.c_str()); + } if (dependFile) { - if (targetFileName.empty()) + if (targetFileName.empty()) { errx("Dependency files can only be created if a target file is specified with either " "-o, -MQ or -MT"); + } fprintf(dependFile, "%s: %s\n", targetFileName.c_str(), mainFileName.c_str()); } @@ -381,8 +409,9 @@ int main(int argc, char *argv[]) { fstk_Init(mainFileName, maxDepth); // Perform parse (`yy::parser` is auto-generated from `parser.y`) - if (yy::parser parser; parser.parse() != 0 && nbErrors == 0) + if (yy::parser parser; parser.parse() != 0 && nbErrors == 0) { nbErrors = 1; + } if (!failedOnMissingInclude) { sect_CheckUnionClosed(); @@ -394,17 +423,20 @@ int main(int argc, char *argv[]) { sect_CheckStack(); } - if (nbErrors != 0) + if (nbErrors != 0) { errx("Assembly aborted (%u error%s)!", nbErrors, nbErrors == 1 ? "" : "s"); + } // If parse aborted due to missing an include, and `-MG` was given, exit normally - if (failedOnMissingInclude) + if (failedOnMissingInclude) { return 0; + } out_WriteObject(); - for (auto [name, features] : stateFileSpecs) + for (auto [name, features] : stateFileSpecs) { out_WriteState(name, features); + } return 0; } diff --git a/src/asm/opt.cpp b/src/asm/opt.cpp index 579168c20..e678a97df 100644 --- a/src/asm/opt.cpp +++ b/src/asm/opt.cpp @@ -53,17 +53,19 @@ void opt_W(char const *flag) { void opt_Parse(char const *s) { switch (s[0]) { case 'b': - if (strlen(&s[1]) == 2) + if (strlen(&s[1]) == 2) { opt_B(&s[1]); - else + } else { error("Must specify exactly 2 characters for option 'b'\n"); + } break; case 'g': - if (strlen(&s[1]) == 4) + if (strlen(&s[1]) == 4) { opt_G(&s[1]); - else + } else { error("Must specify exactly 4 characters for option 'g'\n"); + } break; case 'p': @@ -72,12 +74,13 @@ void opt_Parse(char const *s) { unsigned int padByte; result = sscanf(&s[1], "%x", &padByte); - if (result != 1) + if (result != 1) { error("Invalid argument for option 'p'\n"); - else if (padByte > 0xFF) + } else if (padByte > 0xFF) { error("Argument for option 'p' must be between 0 and 0xFF\n"); - else + } else { opt_P(padByte); + } } else { error("Invalid argument for option 'p'\n"); } @@ -86,19 +89,21 @@ void opt_Parse(char const *s) { char const *precisionArg; case 'Q': precisionArg = &s[1]; - if (precisionArg[0] == '.') + if (precisionArg[0] == '.') { precisionArg++; + } if (strlen(precisionArg) <= 2) { int result; unsigned int precision; result = sscanf(precisionArg, "%u", &precision); - if (result != 1) + if (result != 1) { error("Invalid argument for option 'Q'\n"); - else if (precision < 1 || precision > 31) + } else if (precision < 1 || precision > 31) { error("Argument for option 'Q' must be between 1 and 31\n"); - else + } else { opt_Q(precision); + } } else { error("Invalid argument for option 'Q'\n"); } @@ -106,8 +111,9 @@ void opt_Parse(char const *s) { case 'r': { ++s; // Skip 'r' - while (isblank(*s)) + while (isblank(*s)) { ++s; // Skip leading whitespace + } if (s[0] == '\0') { error("Missing argument to option 'r'\n"); @@ -128,10 +134,11 @@ void opt_Parse(char const *s) { } case 'W': - if (strlen(&s[1]) > 0) + if (strlen(&s[1]) > 0) { opt_W(&s[1]); - else + } else { error("Must specify an argument for option 'W'\n"); + } break; default: diff --git a/src/asm/output.cpp b/src/asm/output.cpp index 5fc57e640..e65b03f06 100644 --- a/src/asm/output.cpp +++ b/src/asm/output.cpp @@ -63,11 +63,13 @@ void out_RegisterNode(std::shared_ptr node) { // Return a section's ID, or UINT32_MAX if the section is not in the list static uint32_t getSectIDIfAny(Section *sect) { - if (!sect) + if (!sect) { return UINT32_MAX; + } - if (auto search = sectionMap.find(sect->name); search != sectionMap.end()) + if (auto search = sectionMap.find(sect->name); search != sectionMap.end()) { return static_cast(search->second); + } fatalerror("Unknown section '%s'\n", sect->name.c_str()); } @@ -109,8 +111,9 @@ static void writeSection(Section const §, FILE *file) { fwrite(sect.data.data(), 1, sect.size, file); putLong(sect.patches.size(), file); - for (Patch const &patch : sect.patches) + for (Patch const &patch : sect.patches) { writePatch(patch, file); + } } } @@ -162,8 +165,9 @@ static void writeRpn(std::vector &rpnexpr, std::vector const & symName.clear(); for (;;) { uint8_t c = rpn[offset++]; - if (c == 0) + if (c == 0) { break; + } symName += c; } @@ -188,8 +192,9 @@ static void writeRpn(std::vector &rpnexpr, std::vector const & symName.clear(); for (;;) { uint8_t c = rpn[offset++]; - if (c == 0) + if (c == 0) { break; + } symName += c; } @@ -298,14 +303,16 @@ static void writeFileStackNode(FileStackNode const &node, FILE *file) { putLong(nodeIters.size(), file); // Iters are stored by decreasing depth, so reverse the order for output - for (uint32_t i = nodeIters.size(); i--;) + for (uint32_t i = nodeIters.size(); i--;) { putLong(nodeIters[i], file); + } } } void out_WriteObject() { - if (objectFileName.empty()) + if (objectFileName.empty()) { return; + } FILE *file; if (objectFileName != "-") { @@ -315,8 +322,9 @@ void out_WriteObject() { (void)setmode(STDOUT_FILENO, O_BINARY); file = stdout; } - if (!file) + if (!file) { err("Failed to open object file '%s'", objectFileName.c_str()); + } Defer closeFile{[&] { fclose(file); }}; // Also write symbols that weren't written above @@ -335,33 +343,39 @@ void out_WriteObject() { writeFileStackNode(node, file); // The list is supposed to have decrementing IDs - if (it + 1 != fileStackNodes.end() && it[1]->ID != node.ID - 1) + if (it + 1 != fileStackNodes.end() && it[1]->ID != node.ID - 1) { fatalerror( "Internal error: fstack node #%" PRIu32 " follows #%" PRIu32 ". Please report this to the developers!\n", it[1]->ID, node.ID ); + } } - for (Symbol const *sym : objectSymbols) + for (Symbol const *sym : objectSymbols) { writeSymbol(*sym, file); + } - for (Section const § : sectionList) + for (Section const § : sectionList) { writeSection(sect, file); + } putLong(assertions.size(), file); - for (Assertion const &assert : assertions) + for (Assertion const &assert : assertions) { writeAssert(assert, file); + } } void out_SetFileName(std::string const &name) { - if (!objectFileName.empty()) + if (!objectFileName.empty()) { warnx("Overriding output filename %s", objectFileName.c_str()); + } objectFileName = name; - if (verbose) + if (verbose) { printf("Output filename %s\n", objectFileName.c_str()); + } } static void dumpString(std::string const &escape, FILE *file) { @@ -397,8 +411,9 @@ static bool dumpEquConstants(FILE *file) { equConstants.clear(); sym_ForEach([](Symbol &sym) { - if (!sym.isBuiltin && sym.type == SYM_EQU) + if (!sym.isBuiltin && sym.type == SYM_EQU) { equConstants.push_back(&sym); + } }); // Constants are ordered by file, then by definition order std::sort(RANGE(equConstants), [](Symbol *sym1, Symbol *sym2) -> bool { @@ -418,8 +433,9 @@ static bool dumpVariables(FILE *file) { variables.clear(); sym_ForEach([](Symbol &sym) { - if (!sym.isBuiltin && sym.type == SYM_VAR) + if (!sym.isBuiltin && sym.type == SYM_VAR) { variables.push_back(&sym); + } }); // Variables are ordered by file, then by definition order std::sort(RANGE(variables), [](Symbol *sym1, Symbol *sym2) -> bool { @@ -439,8 +455,9 @@ static bool dumpEqusConstants(FILE *file) { equsConstants.clear(); sym_ForEach([](Symbol &sym) { - if (!sym.isBuiltin && sym.type == SYM_EQUS) + if (!sym.isBuiltin && sym.type == SYM_EQUS) { equsConstants.push_back(&sym); + } }); // Constants are ordered by file, then by definition order std::sort(RANGE(equsConstants), [](Symbol *sym1, Symbol *sym2) -> bool { @@ -467,8 +484,9 @@ static bool dumpCharmaps(FILE *file) { fputs("charmap \"", charmapFile); dumpString(mapping, charmapFile); putc('"', charmapFile); - for (int32_t v : value) + for (int32_t v : value) { fprintf(charmapFile, ", $%" PRIx32, v); + } putc('\n', charmapFile); } ); @@ -479,8 +497,9 @@ static bool dumpMacros(FILE *file) { macros.clear(); sym_ForEach([](Symbol &sym) { - if (!sym.isBuiltin && sym.type == SYM_MACRO) + if (!sym.isBuiltin && sym.type == SYM_MACRO) { macros.push_back(&sym); + } }); // Macros are ordered by file, then by definition order std::sort(RANGE(macros), [](Symbol *sym1, Symbol *sym2) -> bool { @@ -508,8 +527,9 @@ void out_WriteState(std::string name, std::vector const &features) (void)setmode(STDOUT_FILENO, O_BINARY); file = stdout; } - if (!file) + if (!file) { err("Failed to open state file '%s'", name.c_str()); + } Defer closeFile{[&] { fclose(file); }}; static char const *dumpHeadings[NB_STATE_FEATURES] = { @@ -530,7 +550,8 @@ void out_WriteState(std::string name, std::vector const &features) fputs("; File generated by rgbasm\n", file); for (StateFeature feature : features) { fprintf(file, "\n; %s\n", dumpHeadings[feature]); - if (!dumpFuncs[feature](file)) + if (!dumpFuncs[feature](file)) { fprintf(file, "; No values\n"); + } } } diff --git a/src/asm/rpn.cpp b/src/asm/rpn.cpp index 1bce94227..903002a68 100644 --- a/src/asm/rpn.cpp +++ b/src/asm/rpn.cpp @@ -46,8 +46,9 @@ int32_t Expression::getConstVal() const { } Symbol const *Expression::symbolOf() const { - if (!isSymbol) + if (!isSymbol) { return nullptr; + } return sym_FindScopedSymbol(reinterpret_cast(&rpn[1])); } @@ -55,8 +56,9 @@ bool Expression::isDiffConstant(Symbol const *sym) const { // Check if both expressions only refer to a single symbol Symbol const *sym1 = symbolOf(); - if (!sym1 || !sym || sym1->type != SYM_LABEL || sym->type != SYM_LABEL) + if (!sym1 || !sym || sym1->type != SYM_LABEL || sym->type != SYM_LABEL) { return false; + } Section const *sect1 = sym1->getSection(); Section const *sect2 = sym->getSection(); @@ -208,8 +210,9 @@ static bool tryConstNonzero(Expression const &lhs, Expression const &rhs) { static bool tryConstLogNot(Expression const &expr) { Symbol const *sym = expr.symbolOf(); - if (!sym || !sym->getSection() || !sym->isDefined()) + if (!sym || !sym->getSection() || !sym->isDefined()) { return false; + } assume(sym->isNumeric()); @@ -230,15 +233,17 @@ static bool tryConstLogNot(Expression const &expr) { // @return The constant `LOW(expr)` result if it can be computed, or -1 otherwise. static int32_t tryConstLow(Expression const &expr) { Symbol const *sym = expr.symbolOf(); - if (!sym || !sym->getSection() || !sym->isDefined()) + if (!sym || !sym->getSection() || !sym->isDefined()) { return -1; + } assume(sym->isNumeric()); // The low byte must not cover any unknown bits Section const § = *sym->getSection(); - if (sect.align < 8) + if (sect.align < 8) { return -1; + } // `sym->getValue()` attempts to add the section's address, but that's `UINT32_MAX` // because the section is floating (otherwise we wouldn't be here) @@ -258,15 +263,17 @@ static int32_t tryConstMask(Expression const &lhs, Expression const &rhs) { bool lhsIsSymbol = lhsSymbol && lhsSymbol->getSection(); bool rhsIsSymbol = rhsSymbol && rhsSymbol->getSection(); - if (!lhsIsSymbol && !rhsIsSymbol) + if (!lhsIsSymbol && !rhsIsSymbol) { return -1; + } // If the lhs isn't a symbol, try again the other way around Symbol const &sym = lhsIsSymbol ? *lhsSymbol : *rhsSymbol; Expression const &expr = lhsIsSymbol ? rhs : lhs; // Opposite side of `sym` - if (!sym.isDefined() || !expr.isKnown()) + if (!sym.isDefined() || !expr.isKnown()) { return -1; + } assume(sym.isNumeric()); @@ -275,8 +282,9 @@ static int32_t tryConstMask(Expression const &lhs, Expression const &rhs) { // The mask must not cover any unknown bits Section const § = *sym.getSection(); - if (int32_t unknownBits = (1 << 16) - (1 << sect.align); (unknownBits & mask) != 0) + if (int32_t unknownBits = (1 << 16) - (1 << sect.align); (unknownBits & mask) != 0) { return -1; + } // `sym.getValue()` attempts to add the section's address, but that's `UINT32_MAX` // because the section is floating (otherwise we wouldn't be here) @@ -412,38 +420,45 @@ void Expression::makeBinaryOp(RPNCommand op, Expression &&src1, Expression const data = lval & rval; break; case RPN_SHL: - if (rval < 0) + if (rval < 0) { warning( WARNING_SHIFT_AMOUNT, "Shifting left by negative amount %" PRId32 "\n", rval ); + } - if (rval >= 32) + if (rval >= 32) { warning(WARNING_SHIFT_AMOUNT, "Shifting left by large amount %" PRId32 "\n", rval); + } data = op_shift_left(lval, rval); break; case RPN_SHR: - if (lval < 0) + if (lval < 0) { warning(WARNING_SHIFT, "Shifting right negative value %" PRId32 "\n", lval); + } - if (rval < 0) + if (rval < 0) { warning( WARNING_SHIFT_AMOUNT, "Shifting right by negative amount %" PRId32 "\n", rval ); + } - if (rval >= 32) + if (rval >= 32) { warning(WARNING_SHIFT_AMOUNT, "Shifting right by large amount %" PRId32 "\n", rval); + } data = op_shift_right(lval, rval); break; case RPN_USHR: - if (rval < 0) + if (rval < 0) { warning( WARNING_SHIFT_AMOUNT, "Shifting right by negative amount %" PRId32 "\n", rval ); + } - if (rval >= 32) + if (rval >= 32) { warning(WARNING_SHIFT_AMOUNT, "Shifting right by large amount %" PRId32 "\n", rval); + } data = op_shift_right_unsigned(lval, rval); break; @@ -451,8 +466,9 @@ void Expression::makeBinaryOp(RPNCommand op, Expression &&src1, Expression const data = static_cast(ulval * urval); break; case RPN_DIV: - if (rval == 0) + if (rval == 0) { fatalerror("Division by zero\n"); + } if (lval == INT32_MIN && rval == -1) { warning( @@ -467,17 +483,20 @@ void Expression::makeBinaryOp(RPNCommand op, Expression &&src1, Expression const } break; case RPN_MOD: - if (rval == 0) + if (rval == 0) { fatalerror("Modulo by zero\n"); + } - if (lval == INT32_MIN && rval == -1) + if (lval == INT32_MIN && rval == -1) { data = 0; - else + } else { data = op_modulo(lval, rval); + } break; case RPN_EXP: - if (rval < 0) + if (rval < 0) { fatalerror("Exponentiation by negative power\n"); + } data = op_exponent(lval, rval); break; @@ -554,9 +573,10 @@ void Expression::makeBinaryOp(RPNCommand op, Expression &&src1, Expression const // Copy the right RPN and append the operator uint32_t rightRpnSize = src2.rpn.size(); uint8_t *ptr = reserveSpace(rightRpnSize + 1, src2.rpnPatchSize + 1); - if (rightRpnSize > 0) + if (rightRpnSize > 0) { // If `rightRpnSize == 0`, then `memcpy(ptr, nullptr, rightRpnSize)` would be UB memcpy(ptr, src2.rpn.data(), rightRpnSize); + } ptr[rightRpnSize] = op; } } @@ -589,8 +609,9 @@ void Expression::makeCheckRST() { // Checks that an RPN expression's value fits within N bits (signed or unsigned) void Expression::checkNBit(uint8_t n) const { - if (isKnown()) + if (isKnown()) { ::checkNBit(value(), n, "Expression"); + } } bool checkNBit(int32_t v, uint8_t n, char const *name) { diff --git a/src/asm/section.cpp b/src/asm/section.cpp index 2b17f56d8..ba518ccfb 100644 --- a/src/asm/section.cpp +++ b/src/asm/section.cpp @@ -51,8 +51,9 @@ int32_t loadOffset; // Offset into the LOAD section's parent (see sect_GetOutput // A quick check to see if we have an initialized section [[nodiscard]] static bool requireSection() { - if (currentSection) + if (currentSection) { return true; + } error("Cannot output data outside of a SECTION\n"); return false; @@ -62,11 +63,13 @@ static bool requireSection() { // this much initialized data [[nodiscard]] static bool requireCodeSection() { - if (!requireSection()) + if (!requireSection()) { return false; + } - if (sect_HasData(currentSection->type)) + if (sect_HasData(currentSection->type)) { return true; + } error( "Section '%s' cannot contain code or data (not ROM0 or ROMX)\n", @@ -77,7 +80,7 @@ static bool requireCodeSection() { void sect_CheckSizes() { for (Section const § : sectionList) { - if (uint32_t maxSize = sectionTypeInfo[sect.type].size; sect.size > maxSize) + if (uint32_t maxSize = sectionTypeInfo[sect.type].size; sect.size > maxSize) { error( "Section '%s' grew too big (max size = 0x%" PRIX32 " bytes, reached 0x%" PRIX32 ").\n", @@ -85,6 +88,7 @@ void sect_CheckSizes() { maxSize, sect.size ); + } } } @@ -108,33 +112,36 @@ static unsigned int mergeSectUnion( // Unionized sections only need "compatible" constraints, and they end up with the strictest // combination of both. - if (sect_HasData(type)) + if (sect_HasData(type)) { sectError("Cannot declare ROM sections as UNION\n"); + } if (org != UINT32_MAX) { // If both are fixed, they must be the same - if (sect.org != UINT32_MAX && sect.org != org) + if (sect.org != UINT32_MAX && sect.org != org) { sectError( "Section already declared as fixed at different address $%04" PRIx32 "\n", sect.org ); - else if (sect.align != 0 && (mask(sect.align) & (org - sect.alignOfs))) + } else if (sect.align != 0 && (mask(sect.align) & (org - sect.alignOfs))) { sectError( "Section already declared as aligned to %u bytes (offset %" PRIu16 ")\n", 1U << sect.align, sect.alignOfs ); - else + } else { // Otherwise, just override sect.org = org; + } } else if (alignment != 0) { // Make sure any fixed address given is compatible if (sect.org != UINT32_MAX) { - if ((sect.org - alignOffset) & mask(alignment)) + if ((sect.org - alignOffset) & mask(alignment)) { sectError( "Section already declared as fixed at incompatible address $%04" PRIx32 "\n", sect.org ); + } // Check if alignment offsets are compatible } else if ((alignOffset & mask(sect.align)) != (sect.alignOfs & mask(alignment))) { sectError( @@ -165,34 +172,37 @@ static unsigned int uint16_t curOrg = org - sect.size; // If both are fixed, they must be the same - if (sect.org != UINT32_MAX && sect.org != curOrg) + if (sect.org != UINT32_MAX && sect.org != curOrg) { sectError( "Section already declared as fixed at incompatible address $%04" PRIx32 "\n", sect.org ); - else if (sect.align != 0 && (mask(sect.align) & (curOrg - sect.alignOfs))) + } else if (sect.align != 0 && (mask(sect.align) & (curOrg - sect.alignOfs))) { sectError( "Section already declared as aligned to %u bytes (offset %" PRIu16 ")\n", 1U << sect.align, sect.alignOfs ); - else + } else { // Otherwise, just override sect.org = curOrg; + } } else if (alignment != 0) { int32_t curOfs = (alignOffset - sect.size) % (1U << alignment); - if (curOfs < 0) + if (curOfs < 0) { curOfs += 1U << alignment; + } // Make sure any fixed address given is compatible if (sect.org != UINT32_MAX) { - if ((sect.org - curOfs) & mask(alignment)) + if ((sect.org - curOfs) & mask(alignment)) { sectError( "Section already declared as fixed at incompatible address $%04" PRIx32 "\n", sect.org ); + } // Check if alignment offsets are compatible } else if ((curOfs & mask(sect.align)) != (sect.alignOfs & mask(alignment))) { sectError( @@ -222,10 +232,11 @@ static void mergeSections( ) { unsigned int nbSectErrors = 0; - if (type != sect.type) + if (type != sect.type) { sectError( "Section already exists but with type %s\n", sectionTypeInfo[sect.type].name.c_str() ); + } if (sect.modifier != mod) { sectError("Section already declared as %s section\n", sectionModNames[sect.modifier]); @@ -240,11 +251,13 @@ static void mergeSections( // Common checks // If the section's bank is unspecified, override it - if (sect.bank == UINT32_MAX) + if (sect.bank == UINT32_MAX) { sect.bank = bank; + } // If both specify a bank, it must be the same one - else if (bank != UINT32_MAX && sect.bank != bank) + else if (bank != UINT32_MAX && sect.bank != bank) { sectError("Section already declared with different bank %" PRIu32 "\n", sect.bank); + } break; case SECTION_NORMAL: @@ -255,13 +268,14 @@ static void mergeSections( } } - if (nbSectErrors) + if (nbSectErrors) { fatalerror( "Cannot create section \"%s\" (%u error%s)\n", sect.name.c_str(), nbSectErrors, nbSectErrors == 1 ? "" : "s" ); + } } #undef sectError @@ -294,8 +308,9 @@ static Section *createSection( out_RegisterNode(sect.src); // It is only needed to allocate memory for ROM sections. - if (sect_HasData(type)) + if (sect_HasData(type)) { sect.data.resize(sectionTypeInfo[type].size); + } return § } @@ -316,9 +331,10 @@ static Section *getSection( if (bank != UINT32_MAX) { if (type != SECTTYPE_ROMX && type != SECTTYPE_VRAM && type != SECTTYPE_SRAM - && type != SECTTYPE_WRAMX) + && type != SECTTYPE_WRAMX) { error("BANK only allowed for ROMX, WRAMX, SRAM, or VRAM sections\n"); - else if (bank < sectionTypeInfo[type].firstBank || bank > sectionTypeInfo[type].lastBank) + } else if (bank < sectionTypeInfo[type].firstBank + || bank > sectionTypeInfo[type].lastBank) { error( "%s bank value $%04" PRIx32 " out of range ($%04" PRIx32 " to $%04" PRIx32 ")\n", sectionTypeInfo[type].name.c_str(), @@ -326,6 +342,7 @@ static Section *getSection( sectionTypeInfo[type].firstBank, sectionTypeInfo[type].lastBank ); + } } else if (nbbanks(type) == 1) { // If the section type only has a single bank, implicitly force it bank = sectionTypeInfo[type].firstBank; @@ -341,7 +358,7 @@ static Section *getSection( } if (org != UINT32_MAX) { - if (org < sectionTypeInfo[type].startAddr || org > endaddr(type)) + if (org < sectionTypeInfo[type].startAddr || org > endaddr(type)) { error( "Section \"%s\"'s fixed address $%04" PRIx32 " is outside of range [$%04" PRIx16 "; $%04" PRIx16 "]\n", @@ -350,6 +367,7 @@ static Section *getSection( sectionTypeInfo[type].startAddr, endaddr(type) ); + } } if (alignment != 0) { @@ -361,8 +379,9 @@ static Section *getSection( uint32_t mask = mask(alignment); if (org != UINT32_MAX) { - if ((org - alignOffset) & mask) + if ((org - alignOffset) & mask) { error("Section \"%s\"'s fixed address doesn't match its alignment\n", name.c_str()); + } alignment = 0; // Ignore it if it's satisfied } else if (sectionTypeInfo[type].startAddr & mask) { error( @@ -395,25 +414,29 @@ static Section *getSection( // Set the current section static void changeSection() { - if (!currentUnionStack.empty()) + if (!currentUnionStack.empty()) { fatalerror("Cannot change the section within a UNION\n"); + } sym_ResetCurrentLabelScopes(); } bool Section::isSizeKnown() const { // SECTION UNION and SECTION FRAGMENT can still grow - if (modifier != SECTION_NORMAL) + if (modifier != SECTION_NORMAL) { return false; + } // The current section (or current load section if within one) is still growing - if (this == currentSection || this == currentLoadSection) + if (this == currentSection || this == currentLoadSection) { return false; + } // Any section on the stack is still growing for (SectionStackEntry &entry : sectionStack) { - if (entry.section && entry.section->name == name) + if (entry.section && entry.section->name == name) { return false; + } } return true; @@ -428,12 +451,14 @@ void sect_NewSection( SectionModifier mod ) { for (SectionStackEntry &entry : sectionStack) { - if (entry.section && entry.section->name == name) + if (entry.section && entry.section->name == name) { fatalerror("Section '%s' is already on the stack\n", name.c_str()); + } } - if (currentLoadSection) + if (currentLoadSection) { sect_EndLoadSection("SECTION"); + } Section *sect = getSection(name, type, org, attrs, mod); @@ -456,16 +481,18 @@ void sect_SetLoadSection( // Therefore, any interactions are NOT TESTED, so lift either of those restrictions at // your own peril! ^^ - if (!requireCodeSection()) + if (!requireCodeSection()) { return; + } if (sect_HasData(type)) { error("`LOAD` blocks cannot create a ROM section\n"); return; } - if (currentLoadSection) + if (currentLoadSection) { sect_EndLoadSection("LOAD"); + } Section *sect = getSection(name, type, org, attrs, mod); @@ -477,10 +504,11 @@ void sect_SetLoadSection( } void sect_EndLoadSection(char const *cause) { - if (cause) + if (cause) { warning( WARNING_UNTERMINATED_LOAD, "`LOAD` block without `ENDL` terminated by `%s`\n", cause ); + } if (!currentLoadSection) { error("Found `ENDL` outside of a `LOAD` block\n"); @@ -495,8 +523,9 @@ void sect_EndLoadSection(char const *cause) { } void sect_CheckLoadClosed() { - if (currentLoadSection) + if (currentLoadSection) { warning(WARNING_UNTERMINATED_LOAD, "`LOAD` block without `ENDL` terminated by EOF\n"); + } } Section *sect_GetSymbolSection() { @@ -515,16 +544,18 @@ uint32_t sect_GetOutputOffset() { // Returns how many bytes need outputting for the specified alignment and offset to succeed uint32_t sect_GetAlignBytes(uint8_t alignment, uint16_t offset) { Section *sect = sect_GetSymbolSection(); - if (!sect) + if (!sect) { return 0; + } bool isFixed = sect->org != UINT32_MAX; // If the section is not aligned, no bytes are needed // (fixed sections count as being maximally aligned for this purpose) uint8_t curAlignment = isFixed ? 16 : sect->align; - if (curAlignment == 0) + if (curAlignment == 0) { return 0; + } // We need `(pcValue + curOffset + return value) % (1 << alignment) == offset` uint16_t pcValue = isFixed ? sect->org : sect->alignOfs; @@ -533,18 +564,20 @@ uint32_t sect_GetAlignBytes(uint8_t alignment, uint16_t offset) { } void sect_AlignPC(uint8_t alignment, uint16_t offset) { - if (!requireSection()) + if (!requireSection()) { return; + } Section *sect = sect_GetSymbolSection(); uint32_t alignSize = 1 << alignment; // Size of an aligned "block" if (sect->org != UINT32_MAX) { - if ((sect->org + curOffset - offset) % alignSize) + if ((sect->org + curOffset - offset) % alignSize) { error( "Section's fixed address fails required alignment (PC = $%04" PRIx32 ")\n", sect->org + curOffset ); + } } else if (sect->align != 0 && (((sect->alignOfs + curOffset) % (1u << sect->align)) - offset) % alignSize) { error( @@ -568,18 +601,22 @@ void sect_AlignPC(uint8_t alignment, uint16_t offset) { } static void growSection(uint32_t growth) { - if (growth > 0 && curOffset > UINT32_MAX - growth) + if (growth > 0 && curOffset > UINT32_MAX - growth) { fatalerror("Section size would overflow internal counter\n"); + } curOffset += growth; - if (uint32_t outOffset = sect_GetOutputOffset(); outOffset > currentSection->size) + if (uint32_t outOffset = sect_GetOutputOffset(); outOffset > currentSection->size) { currentSection->size = outOffset; - if (currentLoadSection && curOffset > currentLoadSection->size) + } + if (currentLoadSection && curOffset > currentLoadSection->size) { currentLoadSection->size = curOffset; + } } static void writeByte(uint8_t byte) { - if (uint32_t index = sect_GetOutputOffset(); index < currentSection->data.size()) + if (uint32_t index = sect_GetOutputOffset(); index < currentSection->data.size()) { currentSection->data[index] = byte; + } growSection(1); } @@ -621,8 +658,9 @@ static void endUnionMember() { UnionStackEntry &member = currentUnionStack.top(); uint32_t memberSize = curOffset - member.start; - if (memberSize > member.size) + if (memberSize > member.size) { member.size = memberSize; + } curOffset = member.start; } @@ -645,64 +683,75 @@ void sect_EndUnion() { } void sect_CheckUnionClosed() { - if (!currentUnionStack.empty()) + if (!currentUnionStack.empty()) { error("Unterminated UNION construct\n"); + } } // Output a constant byte void sect_ConstByte(uint8_t byte) { - if (!requireCodeSection()) + if (!requireCodeSection()) { return; + } writeByte(byte); } // Output a string's character units as bytes void sect_ByteString(std::vector const &string) { - if (!requireCodeSection()) + if (!requireCodeSection()) { return; + } for (int32_t unit : string) { - if (!checkNBit(unit, 8, "All character units")) + if (!checkNBit(unit, 8, "All character units")) { break; + } } - for (int32_t unit : string) + for (int32_t unit : string) { writeByte(static_cast(unit)); + } } // Output a string's character units as words void sect_WordString(std::vector const &string) { - if (!requireCodeSection()) + if (!requireCodeSection()) { return; + } for (int32_t unit : string) { - if (!checkNBit(unit, 16, "All character units")) + if (!checkNBit(unit, 16, "All character units")) { break; + } } - for (int32_t unit : string) + for (int32_t unit : string) { writeWord(static_cast(unit)); + } } // Output a string's character units as longs void sect_LongString(std::vector const &string) { - if (!requireCodeSection()) + if (!requireCodeSection()) { return; + } - for (int32_t unit : string) + for (int32_t unit : string) { writeLong(static_cast(unit)); + } } // Skip this many bytes void sect_Skip(uint32_t skip, bool ds) { - if (!requireSection()) + if (!requireSection()) { return; + } if (!sect_HasData(currentSection->type)) { growSection(skip); } else { - if (!ds) + if (!ds) { warning( WARNING_EMPTY_DATA_DIRECTIVE, "%s directive without data in ROM\n", @@ -710,16 +759,19 @@ void sect_Skip(uint32_t skip, bool ds) { : (skip == 2) ? "DW" : "DB" ); + } // We know we're in a code SECTION - while (skip--) + while (skip--) { writeByte(fillByte); + } } } // Output a byte that can be relocatable or constant void sect_RelByte(Expression &expr, uint32_t pcShift) { - if (!requireCodeSection()) + if (!requireCodeSection()) { return; + } if (!expr.isKnown()) { createPatch(PATCHTYPE_BYTE, expr, pcShift); @@ -731,8 +783,9 @@ void sect_RelByte(Expression &expr, uint32_t pcShift) { // Output several bytes that can be relocatable or constant void sect_RelBytes(uint32_t n, std::vector &exprs) { - if (!requireCodeSection()) + if (!requireCodeSection()) { return; + } for (uint32_t i = 0; i < n; i++) { Expression &expr = exprs[i % exprs.size()]; @@ -748,8 +801,9 @@ void sect_RelBytes(uint32_t n, std::vector &exprs) { // Output a word that can be relocatable or constant void sect_RelWord(Expression &expr, uint32_t pcShift) { - if (!requireCodeSection()) + if (!requireCodeSection()) { return; + } if (!expr.isKnown()) { createPatch(PATCHTYPE_WORD, expr, pcShift); @@ -761,8 +815,9 @@ void sect_RelWord(Expression &expr, uint32_t pcShift) { // Output a long that can be relocatable or constant void sect_RelLong(Expression &expr, uint32_t pcShift) { - if (!requireCodeSection()) + if (!requireCodeSection()) { return; + } if (!expr.isKnown()) { createPatch(PATCHTYPE_LONG, expr, pcShift); @@ -774,8 +829,9 @@ void sect_RelLong(Expression &expr, uint32_t pcShift) { // Output a PC-relative byte that can be relocatable or constant void sect_PCRelByte(Expression &expr, uint32_t pcShift) { - if (!requireCodeSection()) + if (!requireCodeSection()) { return; + } if (Symbol const *pc = sym_GetPC(); !expr.isDiffConstant(pc)) { createPatch(PATCHTYPE_JR, expr, pcShift); @@ -786,10 +842,11 @@ void sect_PCRelByte(Expression &expr, uint32_t pcShift) { int16_t offset; // Offset is relative to the byte *after* the operand - if (sym == pc) + if (sym == pc) { offset = -2; // PC as operand to `jr` is lower than reference PC by 2 - else + } else { offset = sym->getValue() - (pc->getValue() + 1); + } if (offset < -128 || offset > 127) { error( @@ -810,16 +867,19 @@ void sect_BinaryFile(std::string const &name, int32_t startPos) { error("Start position cannot be negative (%" PRId32 ")\n", startPos); startPos = 0; } - if (!requireCodeSection()) + if (!requireCodeSection()) { return; + } FILE *file = nullptr; - if (std::optional fullPath = fstk_FindFile(name); fullPath) + if (std::optional fullPath = fstk_FindFile(name); fullPath) { file = fopen(fullPath->c_str(), "rb"); + } if (!file) { if (generatedMissingIncludes) { - if (verbose) + if (verbose) { printf("Aborting (-MG) on INCBIN file '%s' (%s)\n", name.c_str(), strerror(errno)); + } failedOnMissingInclude = true; } else { error("Error opening INCBIN file '%s': %s\n", name.c_str(), strerror(errno)); @@ -836,10 +896,11 @@ void sect_BinaryFile(std::string const &name, int32_t startPos) { // The file is seekable; skip to the specified start position fseek(file, startPos, SEEK_SET); } else { - if (errno != ESPIPE) + if (errno != ESPIPE) { error( "Error determining size of INCBIN file '%s': %s\n", name.c_str(), strerror(errno) ); + } // The file isn't seekable, so we'll just skip bytes one at a time while (startPos--) { if (fgetc(file) == EOF) { @@ -851,11 +912,13 @@ void sect_BinaryFile(std::string const &name, int32_t startPos) { } } - for (int byte; (byte = fgetc(file)) != EOF;) + for (int byte; (byte = fgetc(file)) != EOF;) { writeByte(byte); + } - if (ferror(file)) + if (ferror(file)) { error("Error reading INCBIN file '%s': %s\n", name.c_str(), strerror(errno)); + } } // Output a slice of a binary file @@ -868,18 +931,22 @@ void sect_BinaryFileSlice(std::string const &name, int32_t startPos, int32_t len error("Number of bytes to read cannot be negative (%" PRId32 ")\n", length); length = 0; } - if (!requireCodeSection()) + if (!requireCodeSection()) { return; - if (length == 0) // Don't even bother with 0-byte slices + } + if (length == 0) { // Don't even bother with 0-byte slices return; + } FILE *file = nullptr; - if (std::optional fullPath = fstk_FindFile(name); fullPath) + if (std::optional fullPath = fstk_FindFile(name); fullPath) { file = fopen(fullPath->c_str(), "rb"); + } if (!file) { if (generatedMissingIncludes) { - if (verbose) + if (verbose) { printf("Aborting (-MG) on INCBIN file '%s' (%s)\n", name.c_str(), strerror(errno)); + } failedOnMissingInclude = true; } else { error("Error opening INCBIN file '%s': %s\n", name.c_str(), strerror(errno)); @@ -906,10 +973,11 @@ void sect_BinaryFileSlice(std::string const &name, int32_t startPos, int32_t len // The file is seekable; skip to the specified start position fseek(file, startPos, SEEK_SET); } else { - if (errno != ESPIPE) + if (errno != ESPIPE) { error( "Error determining size of INCBIN file '%s': %s\n", name.c_str(), strerror(errno) ); + } // The file isn't seekable, so we'll just skip bytes one at a time while (startPos--) { if (fgetc(file) == EOF) { @@ -955,11 +1023,13 @@ void sect_PushSection() { } void sect_PopSection() { - if (sectionStack.empty()) + if (sectionStack.empty()) { fatalerror("No entries in the section stack\n"); + } - if (currentLoadSection) + if (currentLoadSection) { sect_EndLoadSection("POPS"); + } SectionStackEntry entry = sectionStack.front(); sectionStack.pop_front(); @@ -980,14 +1050,17 @@ void sect_CheckStack() { } void sect_EndSection() { - if (!currentSection) + if (!currentSection) { fatalerror("Cannot end the section outside of a SECTION\n"); + } - if (!currentUnionStack.empty()) + if (!currentUnionStack.empty()) { fatalerror("Cannot end the section within a UNION\n"); + } - if (currentLoadSection) + if (currentLoadSection) { sect_EndLoadSection("ENDSECTION"); + } // Reset the section scope currentSection = nullptr; diff --git a/src/asm/symbol.cpp b/src/asm/symbol.cpp index 9bb839e3f..0d81f3312 100644 --- a/src/asm/symbol.cpp +++ b/src/asm/symbol.cpp @@ -40,8 +40,9 @@ bool sym_IsPC(Symbol const *sym) { } void sym_ForEach(void (*callback)(Symbol &)) { - for (auto &it : symbols) + for (auto &it : symbols) { callback(it.second); + } } static int32_t NARGCallback() { @@ -101,8 +102,9 @@ std::shared_ptr Symbol::getEqus() const { std::holds_alternative>(data) || std::holds_alternative (*)()>(data) ); - if (auto *callback = std::get_if (*)()>(&data); callback) + if (auto *callback = std::get_if (*)()>(&data); callback) { return (*callback)(); + } return std::get>(data); } @@ -123,8 +125,9 @@ static void updateSymbolFilename(Symbol &sym) { sym.fileLine = sym.src ? lexer_GetLineNo() : 0; // If the old node was registered, ensure the new one is too - if (oldSrc && oldSrc->ID != UINT32_MAX) + if (oldSrc && oldSrc->ID != UINT32_MAX) { out_RegisterNode(sym.src); + } } static void alreadyDefinedError(Symbol const &sym, char const *asType) { @@ -133,8 +136,9 @@ static void alreadyDefinedError(Symbol const &sym, char const *asType) { error("'%s' is reserved for a built-in symbol\n", sym.name.c_str()); } else { error("'%s' already defined", sym.name.c_str()); - if (asType) + if (asType) { fprintf(stderr, " as %s", asType); + } fputs(" at ", stderr); dumpFilename(sym); } @@ -184,28 +188,34 @@ static bool isAutoScoped(std::string const &symName) { size_t dotPos = symName.find('.'); // If there are no dots, it's not a local label - if (dotPos == std::string::npos) + if (dotPos == std::string::npos) { return false; + } // Label scopes `.` and `..` are the only nonlocal identifiers that start with a dot - if (dotPos == 0 && symName.find_first_not_of('.') == symName.npos) + if (dotPos == 0 && symName.find_first_not_of('.') == symName.npos) { return false; + } // Check for nothing after the dot - if (dotPos == symName.length() - 1) + if (dotPos == symName.length() - 1) { fatalerror("'%s' is a nonsensical reference to an empty local label\n", symName.c_str()); + } // Check for more than one dot - if (symName.find('.', dotPos + 1) != std::string::npos) + if (symName.find('.', dotPos + 1) != std::string::npos) { fatalerror("'%s' is a nonsensical reference to a nested local label\n", symName.c_str()); + } // Check for already-qualified local label - if (dotPos > 0) + if (dotPos > 0) { return false; + } // Check for unqualifiable local label - if (!globalScope) + if (!globalScope) { fatalerror("Unqualified local label '%s' in main scope\n", symName.c_str()); + } return true; } @@ -252,24 +262,28 @@ void sym_Purge(std::string const &symName) { Symbol *sym = sym_FindScopedValidSymbol(symName); if (!sym) { - if (sym_IsPurgedScoped(symName)) + if (sym_IsPurgedScoped(symName)) { error("'%s' was already purged\n", symName.c_str()); - else + } else { error("'%s' not defined\n", symName.c_str()); + } } else if (sym->isBuiltin) { error("Built-in symbol '%s' cannot be purged\n", symName.c_str()); } else if (sym->ID != UINT32_MAX) { error("Symbol \"%s\" is referenced and thus cannot be purged\n", symName.c_str()); } else { - if (sym->isExported) + if (sym->isExported) { warning(WARNING_PURGE_1, "Purging an exported symbol \"%s\"\n", symName.c_str()); - else if (sym->isLabel()) + } else if (sym->isLabel()) { warning(WARNING_PURGE_2, "Purging a label \"%s\"\n", symName.c_str()); + } // Do not keep a reference to the label after purging it - if (sym == globalScope) + if (sym == globalScope) { globalScope = nullptr; - if (sym == localScope) + } + if (sym == localScope) { localScope = nullptr; + } purgedSymbols.emplace(sym->name); symbols.erase(sym->name); } @@ -295,14 +309,16 @@ void sym_SetRSValue(int32_t value) { } uint32_t Symbol::getConstantValue() const { - if (isConstant()) + if (isConstant()) { return getValue(); + } if (sym_IsPC(this)) { - if (!getSection()) + if (!getSection()) { error("PC has no value outside of a section\n"); - else + } else { error("PC does not have a constant value; the current section is not fixed\n"); + } } else { error("\"%s\" does not have a constant value\n", name.c_str()); } @@ -310,13 +326,15 @@ uint32_t Symbol::getConstantValue() const { } uint32_t sym_GetConstantValue(std::string const &symName) { - if (Symbol const *sym = sym_FindScopedSymbol(symName); sym) + if (Symbol const *sym = sym_FindScopedSymbol(symName); sym) { return sym->getConstantValue(); + } - if (sym_IsPurgedScoped(symName)) + if (sym_IsPurgedScoped(symName)) { error("'%s' not defined; it was purged\n", symName.c_str()); - else + } else { error("'%s' not defined\n", symName.c_str()); + } return 0; } @@ -361,8 +379,9 @@ static Symbol *createNonrelocSymbol(std::string const &symName, bool numeric) { Symbol *sym_AddEqu(std::string const &symName, int32_t value) { Symbol *sym = createNonrelocSymbol(symName, true); - if (!sym) + if (!sym) { return nullptr; + } sym->type = SYM_EQU; sym->data = value; @@ -373,8 +392,9 @@ Symbol *sym_AddEqu(std::string const &symName, int32_t value) { Symbol *sym_RedefEqu(std::string const &symName, int32_t value) { Symbol *sym = sym_FindExactSymbol(symName); - if (!sym) + if (!sym) { return sym_AddEqu(symName, value); + } if (sym->isDefined() && sym->type != SYM_EQU) { alreadyDefinedError(*sym, "non-EQU"); @@ -394,8 +414,9 @@ Symbol *sym_RedefEqu(std::string const &symName, int32_t value) { Symbol *sym_AddString(std::string const &symName, std::shared_ptr str) { Symbol *sym = createNonrelocSymbol(symName, false); - if (!sym) + if (!sym) { return nullptr; + } sym->type = SYM_EQUS; sym->data = str; @@ -405,8 +426,9 @@ Symbol *sym_AddString(std::string const &symName, std::shared_ptr s Symbol *sym_RedefString(std::string const &symName, std::shared_ptr str) { Symbol *sym = sym_FindExactSymbol(symName); - if (!sym) + if (!sym) { return sym_AddString(symName, str); + } if (sym->type != SYM_EQUS) { if (sym->isDefined()) { @@ -462,12 +484,14 @@ static Symbol *addLabel(std::string const &symName) { sym->type = SYM_LABEL; sym->data = static_cast(sect_GetSymbolOffset()); // Don't export anonymous labels - if (exportAll && !symName.starts_with('!')) + if (exportAll && !symName.starts_with('!')) { sym->isExported = true; + } sym->section = sect_GetSymbolSection(); - if (sym && !sym->section) + if (sym && !sym->section) { error("Label \"%s\" created outside of a SECTION\n", symName.c_str()); + } return sym; } @@ -478,8 +502,9 @@ Symbol *sym_AddLocalLabel(std::string const &symName) { Symbol *sym = addLabel(isAutoScoped(symName) ? globalScope->name + symName : symName); - if (sym) + if (sym) { localScope = sym; + } return sym; } @@ -516,7 +541,7 @@ std::string sym_MakeAnonLabelName(uint32_t ofs, bool neg) { uint32_t id = 0; if (neg) { - if (ofs > anonLabelID) + if (ofs > anonLabelID) { error( "Reference to anonymous label %" PRIu32 " before, when only %" PRIu32 " ha%s been created so far\n", @@ -524,19 +549,21 @@ std::string sym_MakeAnonLabelName(uint32_t ofs, bool neg) { anonLabelID, anonLabelID == 1 ? "s" : "ve" ); - else + } else { id = anonLabelID - ofs; + } } else { ofs--; // We're referencing symbols that haven't been created yet... - if (ofs > UINT32_MAX - anonLabelID) + if (ofs > UINT32_MAX - anonLabelID) { error( "Reference to anonymous label %" PRIu32 " after, when only %" PRIu32 " may still be created\n", ofs + 1, UINT32_MAX - anonLabelID ); - else + } else { id = anonLabelID + ofs; + } } std::string anon("!"); @@ -553,16 +580,18 @@ void sym_Export(std::string const &symName) { Symbol *sym = sym_FindScopedSymbol(symName); // If the symbol doesn't exist, create a ref that can be purged - if (!sym) + if (!sym) { sym = sym_Ref(symName); + } sym->isExported = true; } Symbol *sym_AddMacro(std::string const &symName, int32_t defLineNo, ContentSpan const &span) { Symbol *sym = createNonrelocSymbol(symName, false); - if (!sym) + if (!sym) { return nullptr; + } sym->type = SYM_MACRO; sym->data = span; diff --git a/src/asm/warning.cpp b/src/asm/warning.cpp index 26dc08b37..39a670267 100644 --- a/src/asm/warning.cpp +++ b/src/asm/warning.cpp @@ -85,8 +85,9 @@ enum WarningBehavior { DISABLED, ENABLED, ERROR }; static WarningBehavior getWarningBehavior(WarningID id) { // Check if warnings are globally disabled - if (!warnings) + if (!warnings) { return WarningBehavior::DISABLED; + } // Get the state of this warning flag WarningState const &flagState = warningStates.flagStates[id]; @@ -100,34 +101,43 @@ static WarningBehavior getWarningBehavior(WarningID id) { warningIsError ? WarningBehavior::ERROR : WarningBehavior::ENABLED; // First, check the state of the specific warning flag - if (flagState.state == WARNING_DISABLED) // -Wno- + if (flagState.state == WARNING_DISABLED) { // -Wno- return WarningBehavior::DISABLED; - if (flagState.error == WARNING_ENABLED) // -Werror= + } + if (flagState.error == WARNING_ENABLED) { // -Werror= return WarningBehavior::ERROR; - if (flagState.state == WARNING_ENABLED) // -W + } + if (flagState.state == WARNING_ENABLED) { // -W return enabledBehavior; + } // If no flag is specified, check the state of the "meta" flags that affect this warning flag - if (metaState.state == WARNING_DISABLED) // -Wno- + if (metaState.state == WARNING_DISABLED) { // -Wno- return WarningBehavior::DISABLED; - if (metaState.error == WARNING_ENABLED) // -Werror= + } + if (metaState.error == WARNING_ENABLED) { // -Werror= return WarningBehavior::ERROR; - if (metaState.state == WARNING_ENABLED) // -W + } + if (metaState.state == WARNING_ENABLED) { // -W return enabledBehavior; + } // If no meta flag is specified, check the default state of this warning flag - if (warningFlags[id].level == LEVEL_DEFAULT) // enabled by default + if (warningFlags[id].level == LEVEL_DEFAULT) { // enabled by default return enabledBehavior; + } // No flag enables this warning, explicitly or implicitly return WarningBehavior::DISABLED; } void WarningState::update(WarningState other) { - if (other.state != WARNING_DEFAULT) + if (other.state != WARNING_DEFAULT) { state = other.state; - if (other.error != WARNING_DEFAULT) + } + if (other.error != WARNING_DEFAULT) { error = other.error; + } } void processWarningFlag(char const *flag) { @@ -184,12 +194,14 @@ void processWarningFlag(char const *flag) { // The `if`'s condition above ensures that this will run at least once do { // If we don't have a digit, bail - if (*ptr < '0' || *ptr > '9') + if (*ptr < '0' || *ptr > '9') { break; + } // Avoid overflowing! if (param > UINT8_MAX - (*ptr - '0')) { - if (!warned) + if (!warned) { warnx("Invalid warning flag \"%s\": capping parameter at 255", flag); + } warned = true; // Only warn once, cap always param = 255; continue; @@ -203,8 +215,9 @@ void processWarningFlag(char const *flag) { if (*ptr == '\0') { rootFlag.resize(equals); // `-W=0` is equivalent to `-Wno-` - if (param == 0) + if (param == 0) { state.state = WARNING_DISABLED; + } } } } @@ -218,8 +231,9 @@ void processWarningFlag(char const *flag) { assume(paramWarning.defaultLevel <= maxParam); if (rootFlag == warningFlags[baseID].name) { // Match! - if (rootFlag == "numeric-string") + if (rootFlag == "numeric-string") { warning(WARNING_OBSOLETE, "Warning flag \"numeric-string\" is deprecated\n"); + } // If making the warning an error but param is 0, set to the maximum // This accommodates `-Werror=`, but also `-Werror==0`, which is @@ -229,7 +243,7 @@ void processWarningFlag(char const *flag) { if (param == 0) { param = paramWarning.defaultLevel; } else if (param > maxParam) { - if (param != 255) // Don't warn if already capped + if (param != 255) { // Don't warn if already capped warnx( "Invalid parameter %" PRIu8 " for warning flag \"%s\"; capping at maximum %" PRIu8, @@ -237,16 +251,18 @@ void processWarningFlag(char const *flag) { rootFlag.c_str(), maxParam ); + } param = maxParam; } // Set the first to enabled/error, and disable the rest for (uint8_t ofs = 0; ofs < maxParam; ofs++) { WarningState &warning = warningStates.flagStates[baseID + ofs]; - if (ofs < param) + if (ofs < param) { warning.update(state); - else + } else { warning.state = WARNING_DISABLED; + } } return; } @@ -259,8 +275,9 @@ void processWarningFlag(char const *flag) { if (rootFlag == metaWarning.name) { // Set each of the warning flags that meets this level for (WarningID id : EnumSeq(NB_WARNINGS)) { - if (metaWarning.level >= warningFlags[id].level) + if (metaWarning.level >= warningFlags[id].level) { warningStates.metaStates[id].update(state); + } } return; } @@ -299,13 +316,14 @@ void error(char const *fmt, ...) { // This intentionally makes 0 act as "unlimited" (or at least "limited to sizeof(unsigned)") nbErrors++; - if (nbErrors == maxErrors) + if (nbErrors == maxErrors) { errx( "The maximum of %u error%s was reached (configure with \"-X/--max-errors\"); assembly " "aborted!", maxErrors, maxErrors == 1 ? "" : "s" ); + } } [[noreturn]] diff --git a/src/extern/getopt.cpp b/src/extern/getopt.cpp index 836465f7d..d8bf383b6 100644 --- a/src/extern/getopt.cpp +++ b/src/extern/getopt.cpp @@ -19,8 +19,9 @@ static int musl_optpos; static void musl_getopt_msg(char const *a, char const *b, char const *c, size_t l) { FILE *f = stderr; - if (fputs(a, f) >= 0 && fwrite(b, strlen(b), 1, f) && fwrite(c, 1, l, f) == l) + if (fputs(a, f) >= 0 && fwrite(b, strlen(b), 1, f) && fwrite(c, 1, l, f) == l) { putc('\n', f); + } } static int getopt(int argc, char *argv[], char const *optstring) { @@ -35,8 +36,9 @@ static int getopt(int argc, char *argv[], char const *optstring) { musl_optind = 1; } - if (musl_optind >= argc || !argv[musl_optind]) + if (musl_optind >= argc || !argv[musl_optind]) { return -1; + } if (argv[musl_optind][0] != '-') { if (optstring[0] == '-') { @@ -46,14 +48,17 @@ static int getopt(int argc, char *argv[], char const *optstring) { return -1; } - if (!argv[musl_optind][1]) + if (!argv[musl_optind][1]) { return -1; + } - if (argv[musl_optind][1] == '-' && !argv[musl_optind][2]) + if (argv[musl_optind][1] == '-' && !argv[musl_optind][2]) { return musl_optind++, -1; + } - if (!musl_optpos) + if (!musl_optpos) { musl_optpos++; + } k = mbtowc(&c, argv[musl_optind] + musl_optpos, MB_LEN_MAX); if (k < 0) { k = 1; @@ -67,23 +72,26 @@ static int getopt(int argc, char *argv[], char const *optstring) { musl_optpos = 0; } - if (optstring[0] == '-' || optstring[0] == '+') + if (optstring[0] == '-' || optstring[0] == '+') { optstring++; + } i = 0; d = 0; do { l = mbtowc(&d, optstring + i, MB_LEN_MAX); - if (l > 0) + if (l > 0) { i += l; - else + } else { i++; + } } while (l && d != c); if (d != c || c == ':') { musl_optopt = c; - if (optstring[0] != ':' && musl_opterr) + if (optstring[0] != ':' && musl_opterr) { musl_getopt_msg(argv[0], ": unrecognized option: ", optchar, k); + } return '?'; } if (optstring[i] == ':') { @@ -94,10 +102,12 @@ static int getopt(int argc, char *argv[], char const *optstring) { } if (musl_optind > argc) { musl_optopt = c; - if (optstring[0] == ':') + if (optstring[0] == ':') { return ':'; - if (musl_opterr) + } + if (musl_opterr) { musl_getopt_msg(argv[0], ": option requires an argument: ", optchar, k); + } return '?'; } } @@ -108,8 +118,9 @@ static void permute(char **argv, int dest, int src) { char *tmp = argv[src]; int i; - for (i = src; i > dest; i--) + for (i = src; i > dest; i--) { argv[i] = argv[i - 1]; + } argv[dest] = tmp; } @@ -128,17 +139,20 @@ static int musl_getopt_long( musl_optind = 1; } - if (musl_optind >= argc || !argv[musl_optind]) + if (musl_optind >= argc || !argv[musl_optind]) { return -1; + } skipped = musl_optind; if (optstring[0] != '+' && optstring[0] != '-') { int i; for (i = musl_optind;; i++) { - if (i >= argc || !argv[i]) + if (i >= argc || !argv[i]) { return -1; - if (argv[i][0] == '-' && argv[i][1]) + } + if (argv[i][0] == '-' && argv[i][1]) { break; + } } musl_optind = i; } @@ -147,8 +161,9 @@ static int musl_getopt_long( if (resumed > skipped) { int i, cnt = musl_optind - resumed; - for (i = 0; i < cnt; i++) + for (i = 0; i < cnt; i++) { permute(argv, skipped, musl_optind - 1); + } musl_optind = skipped + cnt; } return ret; @@ -169,14 +184,16 @@ static int musl_getopt_long_core( char const *name = longopts[i].name; opt = start; - if (*opt == '-') + if (*opt == '-') { opt++; + } while (*opt && *opt != '=' && *opt == *name) { name++; opt++; } - if (*opt && *opt != '=') + if (*opt && *opt != '=') { continue; + } arg = opt; match = i; if (!*name) { @@ -191,8 +208,9 @@ static int musl_getopt_long_core( for (i = 0; optstring[i]; i++) { int j = 0; - while (j < l && start[j] == optstring[i + j]) + while (j < l && start[j] == optstring[i + j]) { j++; + } if (j == l) { cnt++; break; @@ -206,8 +224,9 @@ static int musl_getopt_long_core( if (*opt == '=') { if (!longopts[i].has_arg) { musl_optopt = longopts[i].val; - if (colon || !musl_opterr) + if (colon || !musl_opterr) { return '?'; + } musl_getopt_msg( argv[0], ": option does not take an argument: ", @@ -221,10 +240,12 @@ static int musl_getopt_long_core( musl_optarg = argv[musl_optind]; if (!musl_optarg) { musl_optopt = longopts[i].val; - if (colon) + if (colon) { return ':'; - if (!musl_opterr) + } + if (!musl_opterr) { return '?'; + } musl_getopt_msg( argv[0], ": option requires an argument: ", @@ -235,8 +256,9 @@ static int musl_getopt_long_core( } musl_optind++; } - if (idx) + if (idx) { *idx = i; + } if (longopts[i].flag) { *longopts[i].flag = longopts[i].val; return 0; @@ -245,13 +267,14 @@ static int musl_getopt_long_core( } if (argv[musl_optind][1] == '-') { musl_optopt = 0; - if (!colon && musl_opterr) + if (!colon && musl_opterr) { musl_getopt_msg( argv[0], cnt ? ": option is ambiguous: " : ": unrecognized option: ", argv[musl_optind] + 2, strlen(argv[musl_optind] + 2) ); + } musl_optind++; return '?'; } diff --git a/src/fix/main.cpp b/src/fix/main.cpp index 7d0cfe1f3..b8b1c4a1b 100644 --- a/src/fix/main.cpp +++ b/src/fix/main.cpp @@ -83,8 +83,9 @@ static void report(char const *fmt, ...) { vfprintf(stderr, fmt, ap); va_end(ap); - if (nbErrors != UINT8_MAX) + if (nbErrors != UINT8_MAX) { nbErrors++; + } } enum MbcType { @@ -189,16 +190,19 @@ static bool readMBCSlice(char const *&name, char const *expected) { while (*expected) { char c = *name++; - if (c == '\0') // Name too short + if (c == '\0') { // Name too short return false; + } - if (c >= 'a' && c <= 'z') // Perform the comparison case-insensitive + if (c >= 'a' && c <= 'z') { // Perform the comparison case-insensitive c = c - 'a' + 'A'; - else if (c == '_') // Treat underscores as spaces + } else if (c == '_') { // Treat underscores as spaces c = ' '; + } - if (c != *expected++) + if (c != *expected++) { return false; + } } return true; } @@ -221,10 +225,12 @@ static MbcType parseMBC(char const *name) { char *endptr; unsigned long mbc = strtoul(name, &endptr, base); - if (*endptr) + if (*endptr) { return MBC_BAD; - if (mbc > 0xFF) + } + if (mbc > 0xFF) { return MBC_BAD_RANGE; + } return static_cast(mbc); } else { @@ -233,8 +239,9 @@ static MbcType parseMBC(char const *name) { char const *ptr = name; // Trim off leading whitespace - while (*ptr == ' ' || *ptr == '\t') + while (*ptr == ' ' || *ptr == '\t') { ptr++; + } #define tryReadSlice(expected) \ do { \ @@ -247,8 +254,9 @@ static MbcType parseMBC(char const *name) { case 'r': tryReadSlice("OM"); // Handle optional " ONLY" - while (*ptr == ' ' || *ptr == '\t' || *ptr == '_') + while (*ptr == ' ' || *ptr == '\t' || *ptr == '_') { ptr++; + } if (*ptr == 'O' || *ptr == 'o') { ptr++; tryReadSlice("NLY"); @@ -323,8 +331,9 @@ static MbcType parseMBC(char const *name) { case 'P': { tryReadSlice("P1"); // Parse version - while (*ptr == ' ' || *ptr == '_') + while (*ptr == ' ' || *ptr == '_') { ptr++; + } // Major char *endptr; unsigned long val = strtoul(ptr, &endptr, 10); @@ -392,18 +401,22 @@ static MbcType parseMBC(char const *name) { for (;;) { // Trim off trailing whitespace - while (*ptr == ' ' || *ptr == '\t' || *ptr == '_') + while (*ptr == ' ' || *ptr == '\t' || *ptr == '_') { ptr++; + } // If done, start processing "features" - if (!*ptr) + if (!*ptr) { break; + } // We expect a '+' at this point - if (*ptr++ != '+') + if (*ptr++ != '+') { return MBC_BAD; + } // Trim off leading whitespace - while (*ptr == ' ' || *ptr == '\t' || *ptr == '_') + while (*ptr == ' ' || *ptr == '\t' || *ptr == '_') { ptr++; + } switch (*ptr++) { case 'B': // BATTERY @@ -428,8 +441,9 @@ static MbcType parseMBC(char const *name) { break; case 'A': case 'a': - if (*ptr != 'M' && *ptr != 'm') + if (*ptr != 'M' && *ptr != 'm') { return MBC_BAD; + } ptr++; features |= RAM; break; @@ -458,8 +472,9 @@ static MbcType parseMBC(char const *name) { switch (mbc) { case ROM: - if (!features) + if (!features) { break; + } mbc = ROM_RAM - 1; static_assert(ROM_RAM + 1 == ROM_RAM_BATTERY, "Enum sanity check failed!"); static_assert(MBC1 + 1 == MBC1_RAM, "Enum sanity check failed!"); @@ -469,26 +484,29 @@ static MbcType parseMBC(char const *name) { [[fallthrough]]; case MBC1: case MMM01: - if (features == RAM) + if (features == RAM) { mbc++; - else if (features == (RAM | BATTERY)) + } else if (features == (RAM | BATTERY)) { mbc += 2; - else if (features) + } else if (features) { return MBC_WRONG_FEATURES; + } break; case MBC2: - if (features == BATTERY) + if (features == BATTERY) { mbc = MBC2_BATTERY; - else if (features) + } else if (features) { return MBC_WRONG_FEATURES; + } break; case MBC3: // Handle timer, which also requires battery if (features & TIMER) { - if (!(features & BATTERY)) + if (!(features & BATTERY)) { fprintf(stderr, "warning: MBC3+TIMER implies BATTERY\n"); + } features &= ~(TIMER | BATTERY); // Reset those bits mbc = MBC3_TIMER_BATTERY; // RAM is handled below @@ -498,12 +516,13 @@ static MbcType parseMBC(char const *name) { static_assert( MBC3_TIMER_BATTERY + 1 == MBC3_TIMER_RAM_BATTERY, "Enum sanity check failed!" ); - if (features == RAM) + if (features == RAM) { mbc++; - else if (features == (RAM | BATTERY)) + } else if (features == (RAM | BATTERY)) { mbc += 2; - else if (features) + } else if (features) { return MBC_WRONG_FEATURES; + } break; case MBC5: @@ -515,12 +534,13 @@ static MbcType parseMBC(char const *name) { static_assert(MBC5 + 2 == MBC5_RAM_BATTERY, "Enum sanity check failed!"); static_assert(MBC5_RUMBLE + 1 == MBC5_RUMBLE_RAM, "Enum sanity check failed!"); static_assert(MBC5_RUMBLE + 2 == MBC5_RUMBLE_RAM_BATTERY, "Enum sanity check failed!"); - if (features == RAM) + if (features == RAM) { mbc++; - else if (features == (RAM | BATTERY)) + } else if (features == (RAM | BATTERY)) { mbc += 2; - else if (features) + } else if (features) { return MBC_WRONG_FEATURES; + } break; case MBC6: @@ -528,45 +548,56 @@ static MbcType parseMBC(char const *name) { case BANDAI_TAMA5: case HUC3: // No extra features accepted - if (features) + if (features) { return MBC_WRONG_FEATURES; + } break; case MBC7_SENSOR_RUMBLE_RAM_BATTERY: - if (features != (SENSOR | RUMBLE | RAM | BATTERY)) + if (features != (SENSOR | RUMBLE | RAM | BATTERY)) { return MBC_WRONG_FEATURES; + } break; case HUC1_RAM_BATTERY: - if (features != (RAM | BATTERY)) // HuC1 expects RAM+BATTERY + if (features != (RAM | BATTERY)) { // HuC1 expects RAM+BATTERY return MBC_WRONG_FEATURES; + } break; case TPP1: - if (features & RAM) + if (features & RAM) { fprintf( stderr, "warning: TPP1 requests RAM implicitly if given a non-zero RAM size" ); - if (features & BATTERY) + } + if (features & BATTERY) { mbc |= 0x08; - if (features & TIMER) + } + if (features & TIMER) { mbc |= 0x04; - if (features & MULTIRUMBLE) + } + if (features & MULTIRUMBLE) { mbc |= 0x03; // Also set the rumble flag - if (features & RUMBLE) + } + if (features & RUMBLE) { mbc |= 0x01; - if (features & SENSOR) + } + if (features & SENSOR) { return MBC_WRONG_FEATURES; + } break; } // Trim off trailing whitespace - while (*ptr == ' ' || *ptr == '\t') + while (*ptr == ' ' || *ptr == '\t') { ptr++; + } // If there is still something past the whitespace, error out - if (*ptr) + if (*ptr) { return MBC_BAD; + } return static_cast(mbc); } @@ -780,11 +811,13 @@ static ssize_t readBytes(int fd, uint8_t *buf, size_t len) { while (len) { ssize_t ret = read(fd, buf, len); - if (ret == -1 && errno != EINTR) // Return errors, unless we only were interrupted + if (ret == -1 && errno != EINTR) { // Return errors, unless we only were interrupted return -1; + } // EOF reached - if (ret == 0) + if (ret == 0) { return total; + } // If anything was read, accumulate it, and continue if (ret != -1) { total += ret; @@ -805,8 +838,9 @@ static ssize_t writeBytes(int fd, uint8_t *buf, size_t len) { while (len) { ssize_t ret = write(fd, buf, len); - if (ret == -1 && errno != EINTR) // Return errors, unless we only were interrupted + if (ret == -1 && errno != EINTR) { // Return errors, unless we only were interrupted return -1; + } // If anything was written, accumulate it, and continue if (ret != -1) { total += ret; @@ -825,8 +859,9 @@ static ssize_t writeBytes(int fd, uint8_t *buf, size_t len) { static void overwriteByte(uint8_t *rom0, uint16_t addr, uint8_t fixedByte, char const *areaName) { uint8_t origByte = rom0[addr]; - if (!overwriteRom && origByte != 0 && origByte != fixedByte) + if (!overwriteRom && origByte != 0 && origByte != fixedByte) { fprintf(stderr, "warning: Overwrote a non-zero byte in the %s\n", areaName); + } rom0[addr] = fixedByte; } @@ -859,10 +894,11 @@ static void overwriteBytes( // @param fileSize The file's size if known, 0 if not. static void processFile(int input, int output, char const *name, off_t fileSize) { // Both of these should be true for seekable files, and neither otherwise - if (input == output) + if (input == output) { assume(fileSize != 0); - else + } else { assume(fileSize == 0); + } uint8_t rom0[BANK_SIZE]; ssize_t rom0Len = readBytes(input, rom0, sizeof(rom0)); @@ -884,21 +920,25 @@ static void processFile(int input, int output, char const *name, off_t fileSize) } // Accept partial reads if the file contains at least the header - if (fixSpec & (FIX_LOGO | TRASH_LOGO)) + if (fixSpec & (FIX_LOGO | TRASH_LOGO)) { overwriteBytes(rom0, 0x0104, logo, sizeof(logo), logoFilename ? "logo" : "Nintendo logo"); + } - if (title) + if (title) { overwriteBytes(rom0, 0x134, reinterpret_cast(title), titleLen, "title"); + } - if (gameID) + if (gameID) { overwriteBytes( rom0, 0x13F, reinterpret_cast(gameID), gameIDLen, "manufacturer code" ); + } - if (model != DMG) + if (model != DMG) { overwriteByte(rom0, 0x143, model == BOTH ? 0x80 : 0xC0, "CGB flag"); + } - if (newLicensee) + if (newLicensee) { overwriteBytes( rom0, 0x144, @@ -906,9 +946,11 @@ static void processFile(int input, int output, char const *name, off_t fileSize) newLicenseeLen, "new licensee code" ); + } - if (sgb) + if (sgb) { overwriteByte(rom0, 0x146, 0x03, "SGB flag"); + } // If a valid MBC was specified... if (cartridgeType < MBC_NONE) { @@ -931,31 +973,36 @@ static void processFile(int input, int output, char const *name, off_t fileSize) overwriteBytes(rom0, 0x150, tpp1Rev, sizeof(tpp1Rev), "TPP1 revision number"); - if (ramSize != UNSPECIFIED) + if (ramSize != UNSPECIFIED) { overwriteByte(rom0, 0x152, ramSize, "RAM size"); + } overwriteByte(rom0, 0x153, cartridgeType & 0xFF, "TPP1 feature flags"); } else { // Regular mappers - if (ramSize != UNSPECIFIED) + if (ramSize != UNSPECIFIED) { overwriteByte(rom0, 0x149, ramSize, "RAM size"); + } - if (!japanese) + if (!japanese) { overwriteByte(rom0, 0x14A, 0x01, "destination code"); + } } - if (oldLicensee != UNSPECIFIED) + if (oldLicensee != UNSPECIFIED) { overwriteByte(rom0, 0x14B, oldLicensee, "old licensee code"); - else if (sgb && rom0[0x14B] != 0x33) + } else if (sgb && rom0[0x14B] != 0x33) { fprintf( stderr, "warning: SGB compatibility enabled, but old licensee was 0x%02x, not 0x33\n", rom0[0x14B] ); + } - if (romVersion != UNSPECIFIED) + if (romVersion != UNSPECIFIED) { overwriteByte(rom0, 0x14C, romVersion, "mask ROM version number"); + } // Remain to be handled the ROM size, and header checksum. // The latter depends on the former, and so will be handled after it. @@ -1003,13 +1050,15 @@ static void processFile(int input, int output, char const *name, off_t fileSize) nbBanks++; // Update global checksum, too - for (uint16_t i = 0; i < bankLen; i++) + for (uint16_t i = 0; i < bankLen; i++) { globalSum += romx[totalRomxLen + i]; + } totalRomxLen += bankLen; } // Stop when an incomplete bank has been read - if (bankLen != BANK_SIZE) + if (bankLen != BANK_SIZE) { break; + } } } @@ -1036,8 +1085,9 @@ static void processFile(int input, int output, char const *name, off_t fileSize) // Alter number of banks to reflect required value // x&(x-1) is zero iff x is a power of 2, or 0; we know for sure it's non-zero, // so this is true (non-zero) when we don't have a power of 2 - if (nbBanks & (nbBanks - 1)) + if (nbBanks & (nbBanks - 1)) { nbBanks = 1 << (CHAR_BIT * sizeof(nbBanks) - clz(nbBanks)); + } // Write final ROM size rom0[0x148] = ctz(nbBanks / 2); // Alter global checksum based on how many bytes will be added (not counting ROM0) @@ -1048,8 +1098,9 @@ static void processFile(int input, int output, char const *name, off_t fileSize) if (fixSpec & (FIX_HEADER_SUM | TRASH_HEADER_SUM)) { uint8_t sum = 0; - for (uint16_t i = 0x134; i < 0x14D; i++) + for (uint16_t i = 0x134; i < 0x14D; i++) { sum -= rom0[i] + 1; + } overwriteByte(rom0, 0x14D, fixSpec & TRASH_HEADER_SUM ? ~sum : sum, "header checksum"); } @@ -1057,24 +1108,29 @@ static void processFile(int input, int output, char const *name, off_t fileSize) if (fixSpec & (FIX_GLOBAL_SUM | TRASH_GLOBAL_SUM)) { // Computation of the global checksum does not include the checksum bytes assume(rom0Len >= 0x14E); - for (uint16_t i = 0; i < 0x14E; i++) + for (uint16_t i = 0; i < 0x14E; i++) { globalSum += rom0[i]; - for (uint16_t i = 0x150; i < rom0Len; i++) + } + for (uint16_t i = 0x150; i < rom0Len; i++) { globalSum += rom0[i]; + } // Pipes have already read ROMX and updated globalSum, but not regular files if (input == output) { for (;;) { ssize_t bankLen = readBytes(input, bank, sizeof(bank)); - for (uint16_t i = 0; i < bankLen; i++) + for (uint16_t i = 0; i < bankLen; i++) { globalSum += bank[i]; - if (bankLen != sizeof(bank)) + } + if (bankLen != sizeof(bank)) { break; + } } } - if (fixSpec & TRASH_GLOBAL_SUM) + if (fixSpec & TRASH_GLOBAL_SUM) { globalSum = ~globalSum; + } uint8_t bytes[2] = { static_cast(globalSum >> 8), static_cast(globalSum & 0xFF) @@ -1094,8 +1150,9 @@ static void processFile(int input, int output, char const *name, off_t fileSize) } // If modifying the file in-place, we only need to edit the header // However, padding may have modified ROM0 (added padding), so don't in that case - if (padValue == UNSPECIFIED) + if (padValue == UNSPECIFIED) { rom0Len = headerSize; + } } writeLen = writeBytes(output, rom0, rom0Len); @@ -1198,7 +1255,7 @@ static bool processFilename(char const *name) { } } - if (nbErrors) + if (nbErrors) { fprintf( stderr, "Fixing \"%s\" failed with %u error%s\n", @@ -1206,6 +1263,7 @@ static bool processFilename(char const *name) { nbErrors, nbErrors == 1 ? "" : "s" ); + } return nbErrors; } @@ -1386,21 +1444,23 @@ int main(int argc, char *argv[]) { } } - if ((cartridgeType & 0xFF00) == TPP1 && !japanese) + if ((cartridgeType & 0xFF00) == TPP1 && !japanese) { fprintf( stderr, "warning: TPP1 overwrites region flag for its identification code, ignoring `-j`\n" ); + } // Check that RAM size is correct for "standard" mappers if (ramSize != UNSPECIFIED && (cartridgeType & 0xFF00) == 0) { if (cartridgeType == ROM_RAM || cartridgeType == ROM_RAM_BATTERY) { - if (ramSize != 1) + if (ramSize != 1) { fprintf( stderr, "warning: MBC \"%s\" should have 2 KiB of RAM (-r 1)\n", mbcName(cartridgeType) ); + } } else if (hasRAM(cartridgeType)) { if (!ramSize) { fprintf( @@ -1425,12 +1485,13 @@ int main(int argc, char *argv[]) { } } - if (sgb && oldLicensee != UNSPECIFIED && oldLicensee != 0x33) + if (sgb && oldLicensee != UNSPECIFIED && oldLicensee != 0x33) { fprintf( stderr, "warning: SGB compatibility enabled, but old licensee is 0x%02x, not 0x33\n", oldLicensee ); + } argv += musl_optind; bool failed = nbErrors; @@ -1481,8 +1542,9 @@ int main(int argc, char *argv[]) { memcpy(logo, nintendoLogo, sizeof(nintendoLogo)); } if (fixSpec & TRASH_LOGO) { - for (uint16_t i = 0; i < sizeof(logo); i++) + for (uint16_t i = 0; i < sizeof(logo); i++) { logo[i] = 0xFF ^ logo[i]; + } } if (!*argv) { diff --git a/src/gfx/main.cpp b/src/gfx/main.cpp index 1e7462e85..c78983096 100644 --- a/src/gfx/main.cpp +++ b/src/gfx/main.cpp @@ -72,15 +72,17 @@ void error(char const *fmt, ...) { va_end(ap); putc('\n', stderr); - if (nbErrors != std::numeric_limits::max()) + if (nbErrors != std::numeric_limits::max()) { nbErrors++; + } } void errorMessage(char const *msg) { fprintf(stderr, "error: %s\n", msg); - if (nbErrors != std::numeric_limits::max()) + if (nbErrors != std::numeric_limits::max()) { nbErrors++; + } } [[noreturn]] @@ -93,8 +95,9 @@ void fatal(char const *fmt, ...) { va_end(ap); putc('\n', stderr); - if (nbErrors != std::numeric_limits::max()) + if (nbErrors != std::numeric_limits::max()) { nbErrors++; + } giveUp(); } @@ -354,8 +357,9 @@ static char *parseArgv(int argc, char *argv[]) { break; case 'a': localOptions.autoAttrmap = false; - if (!options.attrmap.empty()) + if (!options.attrmap.empty()) { warning("Overriding attrmap file %s", options.attrmap.c_str()); + } options.attrmap = musl_optarg; break; case 'b': @@ -425,8 +429,9 @@ static char *parseArgv(int argc, char *argv[]) { printUsage(); exit(0); case 'i': - if (!options.inputTileset.empty()) + if (!options.inputTileset.empty()) { warning("Overriding input tileset file %s", options.inputTileset.c_str()); + } options.inputTileset = musl_optarg; break; case 'L': @@ -524,8 +529,9 @@ static char *parseArgv(int argc, char *argv[]) { localOptions.groupOutputs = true; break; case 'o': - if (!options.output.empty()) + if (!options.output.empty()) { warning("Overriding tile data file %s", options.output.c_str()); + } options.output = musl_optarg; break; case 'P': @@ -533,8 +539,9 @@ static char *parseArgv(int argc, char *argv[]) { break; case 'p': localOptions.autoPalettes = false; - if (!options.palettes.empty()) + if (!options.palettes.empty()) { warning("Overriding palettes file %s", options.palettes.c_str()); + } options.palettes = musl_optarg; break; case 'Q': @@ -542,8 +549,9 @@ static char *parseArgv(int argc, char *argv[]) { break; case 'q': localOptions.autoPalmap = false; - if (!options.palmap.empty()) + if (!options.palmap.empty()) { warning("Overriding palette map file %s", options.palmap.c_str()); + } options.palmap = musl_optarg; break; case 'r': @@ -569,8 +577,9 @@ static char *parseArgv(int argc, char *argv[]) { break; case 't': localOptions.autoTilemap = false; - if (!options.tilemap.empty()) + if (!options.tilemap.empty()) { warning("Overriding tilemap file %s", options.tilemap.c_str()); + } options.tilemap = musl_optarg; break; case 'V': @@ -768,19 +777,25 @@ int main(int argc, char *argv[]) { } fputs("Options:\n", stderr); - if (options.columnMajor) + if (options.columnMajor) { fputs("\tVisit image in column-major order\n", stderr); - if (options.allowDedup) + } + if (options.allowDedup) { fputs("\tAllow deduplicating tiles\n", stderr); - if (options.allowMirroringX) + } + if (options.allowMirroringX) { fputs("\tAllow deduplicating horizontally mirrored tiles\n", stderr); - if (options.allowMirroringY) + } + if (options.allowMirroringY) { fputs("\tAllow deduplicating vertically mirrored tiles\n", stderr); - if (options.useColorCurve) + } + if (options.useColorCurve) { fputs("\tUse color curve\n", stderr); + } fprintf(stderr, "\tBit depth: %" PRIu8 "bpp\n", options.bitDepth); - if (options.trim != 0) + if (options.trim != 0) { fprintf(stderr, "\tTrim the last %" PRIu64 " tiles\n", options.trim); + } fprintf(stderr, "\tMaximum %" PRIu16 " palettes\n", options.nbPalettes); fprintf(stderr, "\tPalettes contain %" PRIu8 " colors\n", options.nbColorsPerPal); fprintf(stderr, "\t%s palette spec\n", [] { diff --git a/src/gfx/reverse.cpp b/src/gfx/reverse.cpp index 74d37f8f2..d2dbd1890 100644 --- a/src/gfx/reverse.cpp +++ b/src/gfx/reverse.cpp @@ -167,8 +167,9 @@ void reverse() { // This avoids redundancy with `-r 1` which results in a vertical column. width = static_cast(ceil(sqrt(mapSize))); for (; width < mapSize; ++width) { - if (mapSize % width == 0) + if (mapSize % width == 0) { break; + } } options.verbosePrint(Options::VERB_INTERM, "Picked reversing width of %zu tiles\n", width); } diff --git a/src/link/assign.cpp b/src/link/assign.cpp index 6cebffe08..b0220b527 100644 --- a/src/link/assign.cpp +++ b/src/link/assign.cpp @@ -74,14 +74,17 @@ static void assignSection(Section §ion, MemoryLocation const &location) { static bool isLocationSuitable( Section const §ion, FreeSpace const &freeSpace, MemoryLocation const &location ) { - if (section.isAddressFixed && section.org != location.address) + if (section.isAddressFixed && section.org != location.address) { return false; + } - if (section.isAlignFixed && ((location.address - section.alignOfs) & section.alignMask)) + if (section.isAlignFixed && ((location.address - section.alignOfs) & section.alignMask)) { return false; + } - if (location.address < freeSpace.address) + if (location.address < freeSpace.address) { return false; + } return location.address + section.size <= freeSpace.address + freeSpace.size; } @@ -102,16 +105,19 @@ static ssize_t getPlacement(Section const §ion, MemoryLocation &location) { if (section.isBankFixed) { location.bank = section.bank; } else if (scrambleROMX && section.type == SECTTYPE_ROMX) { - if (curScrambleROM < 1) + if (curScrambleROM < 1) { curScrambleROM = scrambleROMX; + } location.bank = curScrambleROM--; } else if (scrambleWRAMX && section.type == SECTTYPE_WRAMX) { - if (curScrambleWRAM < 1) + if (curScrambleWRAM < 1) { curScrambleWRAM = scrambleWRAMX; + } location.bank = curScrambleWRAM--; } else if (scrambleSRAM && section.type == SECTTYPE_SRAM) { - if (curScrambleSRAM < 0) + if (curScrambleSRAM < 0) { curScrambleSRAM = scrambleSRAM; + } location.bank = curScrambleSRAM--; } else { location.bank = typeInfo.firstBank; @@ -128,18 +134,20 @@ static ssize_t getPlacement(Section const §ion, MemoryLocation &location) { // Process locations in that bank while (spaceIdx < bankMem.size()) { // If that location is OK, return it - if (isLocationSuitable(section, bankMem[spaceIdx], location)) + if (isLocationSuitable(section, bankMem[spaceIdx], location)) { return spaceIdx; + } // Go to the next *possible* location if (section.isAddressFixed) { // If the address is fixed, there can be only // one candidate block per bank; if we already // reached it, give up. - if (location.address < section.org) + if (location.address < section.org) { location.address = section.org; - else + } else { break; // Try again in next bank + } } else if (section.isAlignFixed) { // Move to next aligned location // Move back to alignment boundary @@ -151,15 +159,17 @@ static ssize_t getPlacement(Section const §ion, MemoryLocation &location) { } else { // Any location is fine, so, next free block spaceIdx++; - if (spaceIdx < bankMem.size()) + if (spaceIdx < bankMem.size()) { location.address = bankMem[spaceIdx].address; + } } // If that location is past the current block's end, // go forwards until that is no longer the case. while (spaceIdx < bankMem.size() - && location.address >= bankMem[spaceIdx].address + bankMem[spaceIdx].size) + && location.address >= bankMem[spaceIdx].address + bankMem[spaceIdx].size) { spaceIdx++; + } // Try again with the new location/free space combo } @@ -171,27 +181,30 @@ static ssize_t getPlacement(Section const §ion, MemoryLocation &location) { if (section.isBankFixed) { return -1; } else if (scrambleROMX && section.type == SECTTYPE_ROMX && location.bank <= scrambleROMX) { - if (location.bank > typeInfo.firstBank) + if (location.bank > typeInfo.firstBank) { location.bank--; - else if (scrambleROMX < typeInfo.lastBank) + } else if (scrambleROMX < typeInfo.lastBank) { location.bank = scrambleROMX + 1; - else + } else { return -1; + } } else if (scrambleWRAMX && section.type == SECTTYPE_WRAMX && location.bank <= scrambleWRAMX) { - if (location.bank > typeInfo.firstBank) + if (location.bank > typeInfo.firstBank) { location.bank--; - else if (scrambleWRAMX < typeInfo.lastBank) + } else if (scrambleWRAMX < typeInfo.lastBank) { location.bank = scrambleWRAMX + 1; - else + } else { return -1; + } } else if (scrambleSRAM && section.type == SECTTYPE_SRAM && location.bank <= scrambleSRAM) { - if (location.bank > typeInfo.firstBank) + if (location.bank > typeInfo.firstBank) { location.bank--; - else if (scrambleSRAM < typeInfo.lastBank) + } else if (scrambleSRAM < typeInfo.lastBank) { location.bank = scrambleSRAM + 1; - else + } else { return -1; + } } else if (location.bank < typeInfo.lastBank) { location.bank++; } else { @@ -251,9 +264,10 @@ static void placeSection(Section §ion) { } else { // The amount of free spaces doesn't change: resize! freeSpace.size -= section.size; - if (noLeftSpace) + if (noLeftSpace) { // The free space is moved *and* resized freeSpace.address += section.size; + } } return; } @@ -262,11 +276,11 @@ static void placeSection(Section §ion) { char where[64]; if (section.isBankFixed && nbbanks(section.type) != 1) { - if (section.isAddressFixed) + if (section.isAddressFixed) { snprintf( where, sizeof(where), "at $%02" PRIx32 ":%04" PRIx16, section.bank, section.org ); - else if (section.isAlignFixed) + } else if (section.isAlignFixed) { snprintf( where, sizeof(where), @@ -274,12 +288,13 @@ static void placeSection(Section §ion) { section.bank, static_cast(~section.alignMask) ); - else + } else { snprintf(where, sizeof(where), "in bank $%02" PRIx32, section.bank); + } } else { - if (section.isAddressFixed) + if (section.isAddressFixed) { snprintf(where, sizeof(where), "at address $%04" PRIx16, section.org); - else if (section.isAlignFixed) + } else if (section.isAlignFixed) { snprintf( where, sizeof(where), @@ -287,20 +302,22 @@ static void placeSection(Section §ion) { static_cast(~section.alignMask), section.alignOfs ); - else + } else { strcpy(where, "anywhere"); + } } // If a section failed to go to several places, nothing we can report - if (!section.isBankFixed || !section.isAddressFixed) + if (!section.isBankFixed || !section.isAddressFixed) { errx( "Unable to place \"%s\" (%s section) %s", section.name.c_str(), sectionTypeInfo[section.type].name.c_str(), where ); + } // If the section just can't fit the bank, report that - else if (section.org + section.size > endaddr(section.type) + 1) + else if (section.org + section.size > endaddr(section.type) + 1) { errx( "Unable to place \"%s\" (%s section) %s: section runs past end of region ($%04x > " "$%04x)", @@ -310,8 +327,9 @@ static void placeSection(Section §ion) { section.org + section.size, endaddr(section.type) + 1 ); + } // Otherwise there is overlap with another section - else + else { errx( "Unable to place \"%s\" (%s section) %s: section overlaps with \"%s\"", section.name.c_str(), @@ -319,6 +337,7 @@ static void placeSection(Section §ion) { where, out_OverlappingSection(section)->name.c_str() ); + } } // clang-format off: vertically align values @@ -334,20 +353,24 @@ static std::deque
unassignedSections[1 << 3]; static void categorizeSection(Section §ion) { uint8_t constraints = 0; - if (section.isBankFixed) + if (section.isBankFixed) { constraints |= BANK_CONSTRAINED; - if (section.isAddressFixed) + } + if (section.isAddressFixed) { constraints |= ORG_CONSTRAINED; + } // Can't have both! - else if (section.isAlignFixed) + else if (section.isAlignFixed) { constraints |= ALIGN_CONSTRAINED; + } std::deque
§ions = unassignedSections[constraints]; auto pos = sections.begin(); // Insert section while keeping the list sorted by decreasing size - while (pos != sections.end() && (*pos)->size > section.size) + while (pos != sections.end() && (*pos)->size > section.size) { pos++; + } sections.insert(pos, §ion); nbSectionsToAssign++; @@ -368,12 +391,14 @@ void assign_AssignSections() { // Specially process fully-constrained sections because of overlaying verbosePrint("Assigning bank+org-constrained...\n"); - for (Section *section : unassignedSections[BANK_CONSTRAINED | ORG_CONSTRAINED]) + for (Section *section : unassignedSections[BANK_CONSTRAINED | ORG_CONSTRAINED]) { placeSection(*section); + } // If all sections were fully constrained, we have nothing left to do - if (!nbSectionsToAssign) + if (!nbSectionsToAssign) { return; + } // Overlaying requires only fully-constrained sections verbosePrint("Assigning other sections...\n"); @@ -385,14 +410,16 @@ void assign_AssignSections() { for (Section *section : unassignedSections[constraints]) { fprintf(stderr, "%c \"%s\"", nbSections == 0 ? ';' : ',', section->name.c_str()); nbSections++; - if (nbSections == 10) + if (nbSections == 10) { goto max_out; // Can't `break` out of a nested loop + } } } max_out: - if (nbSectionsToAssign != nbSections) + if (nbSectionsToAssign != nbSections) { fprintf(stderr, " and %" PRIu64 " more", nbSectionsToAssign - nbSections); + } fprintf(stderr, " %sn't\n", nbSectionsToAssign == 1 ? "is" : "are"); exit(1); } @@ -400,11 +427,13 @@ void assign_AssignSections() { // Assign all remaining sections by decreasing constraint order for (int8_t constraints = BANK_CONSTRAINED | ALIGN_CONSTRAINED; constraints >= 0; constraints--) { - for (Section *section : unassignedSections[constraints]) + for (Section *section : unassignedSections[constraints]) { placeSection(*section); + } - if (!nbSectionsToAssign) + if (!nbSectionsToAssign) { return; + } } unreachable_(); diff --git a/src/link/main.cpp b/src/link/main.cpp index f3444e9ed..0fd54eca9 100644 --- a/src/link/main.cpp +++ b/src/link/main.cpp @@ -52,8 +52,9 @@ std::string const &FileStackNode::dump(uint32_t curLineNo) const { std::string const &lastName = parent->dump(lineNo); fputs(" -> ", stderr); fputs(lastName.c_str(), stderr); - for (uint32_t iter : iters()) + for (uint32_t iter : iters()) { fprintf(stderr, "::REPT~%" PRIu32, iter); + } fprintf(stderr, "(%" PRIu32 ")", curLineNo); return lastName; } else { @@ -96,8 +97,9 @@ void error(FileStackNode const *where, uint32_t lineNo, char const *fmt, ...) { printDiag(fmt, args, "error", where, lineNo); va_end(args); - if (nbErrors != UINT32_MAX) + if (nbErrors != UINT32_MAX) { nbErrors++; + } } void argErr(char flag, char const *fmt, ...) { @@ -109,8 +111,9 @@ void argErr(char flag, char const *fmt, ...) { va_end(args); putc('\n', stderr); - if (nbErrors != UINT32_MAX) + if (nbErrors != UINT32_MAX) { nbErrors++; + } } [[noreturn]] @@ -121,8 +124,9 @@ void fatal(FileStackNode const *where, uint32_t lineNo, char const *fmt, ...) { printDiag(fmt, args, "FATAL", where, lineNo); va_end(args); - if (nbErrors != UINT32_MAX) + if (nbErrors != UINT32_MAX) { nbErrors++; + } fprintf( stderr, "Linking aborted after %" PRIu32 " error%s\n", nbErrors, nbErrors == 1 ? "" : "s" @@ -216,10 +220,12 @@ static void parseScrambleSpec(char const *spec) { if (regionNameLen == 0) { argErr('S', "Missing region name"); - if (*spec == '\0') + if (*spec == '\0') { break; - if (*spec == '=') // Skip the limit, too + } + if (*spec == '=') { // Skip the limit, too spec = strchr(&spec[1], ','); // Skip to next comma, if any + } goto next; } @@ -242,8 +248,9 @@ static void parseScrambleSpec(char const *spec) { } } - if (region == SCRAMBLE_UNK) + if (region == SCRAMBLE_UNK) { argErr('S', "Unknown region \"%.*s\"", regionNameFmtLen, regionName); + } if (*spec == '=') { spec++; // `strtoul` will skip the whitespace on its own @@ -301,10 +308,12 @@ static void parseScrambleSpec(char const *spec) { next: // Can't `continue` a `for` loop with this nontrivial iteration logic if (spec) { assume(*spec == ',' || *spec == '\0'); - if (*spec == ',') + if (*spec == ',') { spec += 1 + strspn(&spec[1], " \t"); - if (*spec == '\0') + } + if (*spec == '\0') { break; + } } } } @@ -329,31 +338,36 @@ int main(int argc, char *argv[]) { printUsage(); exit(0); case 'l': - if (linkerScriptName) + if (linkerScriptName) { warnx("Overriding linker script %s", linkerScriptName); + } linkerScriptName = musl_optarg; break; case 'M': noSymInMap = true; break; case 'm': - if (mapFileName) + if (mapFileName) { warnx("Overriding map file %s", mapFileName); + } mapFileName = musl_optarg; break; case 'n': - if (symFileName) + if (symFileName) { warnx("Overriding sym file %s", symFileName); + } symFileName = musl_optarg; break; case 'O': - if (overlayFileName) + if (overlayFileName) { warnx("Overriding overlay file %s", overlayFileName); + } overlayFileName = musl_optarg; break; case 'o': - if (outputFileName) + if (outputFileName) { warnx("Overriding output file %s", outputFileName); + } outputFileName = musl_optarg; break; case 'p': { @@ -410,18 +424,22 @@ int main(int argc, char *argv[]) { } // Patch the size array depending on command-line options - if (!is32kMode) + if (!is32kMode) { sectionTypeInfo[SECTTYPE_ROM0].size = 0x4000; - if (!isWRAM0Mode) + } + if (!isWRAM0Mode) { sectionTypeInfo[SECTTYPE_WRAM0].size = 0x1000; + } // Patch the bank ranges array depending on command-line options - if (isDmgMode) + if (isDmgMode) { sectionTypeInfo[SECTTYPE_VRAM].lastBank = 0; + } // Read all object files first, - for (obj_Setup(argc - curArgIndex); curArgIndex < argc; curArgIndex++) + for (obj_Setup(argc - curArgIndex); curArgIndex < argc; curArgIndex++) { obj_ReadFile(argv[curArgIndex], argc - curArgIndex - 1); + } // apply the linker script's modifications, if (linkerScriptName) { @@ -430,20 +448,23 @@ int main(int argc, char *argv[]) { script_ProcessScript(linkerScriptName); // If the linker script produced any errors, some sections may be in an invalid state - if (nbErrors != 0) + if (nbErrors != 0) { reportErrors(); + } } // then process them, sect_DoSanityChecks(); - if (nbErrors != 0) + if (nbErrors != 0) { reportErrors(); + } assign_AssignSections(); patch_CheckAssertions(); // and finally output the result. patch_ApplyPatches(); - if (nbErrors != 0) + if (nbErrors != 0) { reportErrors(); + } out_WriteFiles(); } diff --git a/src/link/object.cpp b/src/link/object.cpp index 89b023ef4..7a77b345f 100644 --- a/src/link/object.cpp +++ b/src/link/object.cpp @@ -51,8 +51,9 @@ static int64_t readLong(FILE *file) { for (uint8_t shift = 0; shift < sizeof(value) * CHAR_BIT; shift += 8) { int byte = getc(file); - if (byte == EOF) + if (byte == EOF) { return INT64_MAX; + } // This must be casted to `unsigned`, not `uint8_t`. Rationale: // the type of the shift is the type of `byte` after undergoing // integer promotion, which would be `int` if this was casted to @@ -141,7 +142,7 @@ static void readFileStackNode( case NODE_REPT: tryReadLong(depth, file, "%s: Cannot read node #%" PRIu32 "'s rept depth: %s", fileName, i); node.data = std::vector(depth); - for (uint32_t k = 0; k < depth; k++) + for (uint32_t k = 0; k < depth; k++) { tryReadLong( node.iters()[k], file, @@ -150,7 +151,8 @@ static void readFileStackNode( i, k ); - if (!node.parent) + } + if (!node.parent) { fatal( nullptr, 0, @@ -158,6 +160,7 @@ static void readFileStackNode( fileName, i ); + } } } @@ -296,7 +299,7 @@ static void readPatch( patch.rpnExpression.resize(rpnSize); size_t nbElementsRead = fread(patch.rpnExpression.data(), 1, rpnSize, file); - if (nbElementsRead != rpnSize) + if (nbElementsRead != rpnSize) { errx( "%s: Cannot read \"%s\"'s patch #%" PRIu32 "'s RPN expression: %s", fileName, @@ -304,6 +307,7 @@ static void readPatch( i, feof(file) ? "Unexpected end of file" : strerror(errno) ); + } } // Sets a patch's pcSection from its pcSectionID. @@ -338,8 +342,9 @@ static void readSection( section.name.c_str() ); tryReadLong(tmp, file, "%s: Cannot read \"%s\"'s' size: %s", fileName, section.name.c_str()); - if (tmp < 0 || tmp > UINT16_MAX) + if (tmp < 0 || tmp > UINT16_MAX) { errx("\"%s\"'s section size ($%" PRIx32 ") is invalid", section.name.c_str(), tmp); + } section.size = tmp; section.offset = 0; tryGetc( @@ -350,12 +355,13 @@ static void readSection( } else { section.type = SectionType(type); } - if (byte >> 7) + if (byte >> 7) { section.modifier = SECTION_UNION; - else if (byte >> 6) + } else if (byte >> 6) { section.modifier = SECTION_FRAGMENT; - else + } else { section.modifier = SECTION_NORMAL; + } tryReadLong(tmp, file, "%s: Cannot read \"%s\"'s org: %s", fileName, section.name.c_str()); section.isAddressFixed = tmp >= 0; if (tmp > UINT16_MAX) { @@ -374,8 +380,9 @@ static void readSection( fileName, section.name.c_str() ); - if (byte > 16) + if (byte > 16) { byte = 16; + } section.isAlignFixed = byte != 0; section.alignMask = (1 << byte) - 1; tryReadLong( @@ -397,13 +404,14 @@ static void readSection( if (section.size) { section.data.resize(section.size); if (size_t nbRead = fread(section.data.data(), 1, section.size, file); - nbRead != section.size) + nbRead != section.size) { errx( "%s: Cannot read \"%s\"'s data: %s", fileName, section.name.c_str(), feof(file) ? "Unexpected end of file" : strerror(errno) ); + } } uint32_t nbPatches; @@ -417,8 +425,9 @@ static void readSection( ); section.patches.resize(nbPatches); - for (uint32_t i = 0; i < nbPatches; i++) + for (uint32_t i = 0; i < nbPatches; i++) { readPatch(file, section.patches[i], fileName, section.name, i, fileNodes); + } } } @@ -433,10 +442,11 @@ static void linkSymToSect(Symbol &symbol, Section §ion) { uint32_t c = (a + b) / 2; int32_t otherOffset = section.symbols[c]->label().offset; - if (otherOffset > symbolOffset) + if (otherOffset > symbolOffset) { b = c; - else + } else { a = c + 1; + } } section.symbols.insert(section.symbols.begin() + a, &symbol); @@ -469,8 +479,9 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) { (void)setmode(STDIN_FILENO, O_BINARY); file = stdin; } - if (!file) + 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', @@ -506,15 +517,16 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) { int matchedElems; if (fscanf(file, RGBDS_OBJECT_VERSION_STRING "%n", &matchedElems) == 1 - && matchedElems != literal_strlen(RGBDS_OBJECT_VERSION_STRING)) + && matchedElems != literal_strlen(RGBDS_OBJECT_VERSION_STRING)) { errx("%s: Not a RGBDS object file", fileName); + } verbosePrint("Reading object file %s\n", fileName); uint32_t revNum; tryReadLong(revNum, file, "%s: Cannot read revision number: %s", fileName); - if (revNum != RGBDS_OBJECT_REV) + if (revNum != RGBDS_OBJECT_REV) { errx( "%s: Unsupported object file for rgblink %s; try rebuilding \"%s\"%s" " (expected revision %d, got %d)", @@ -525,6 +537,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) { RGBDS_OBJECT_REV, revNum ); + } uint32_t nbNodes; uint32_t nbSymbols; @@ -538,8 +551,9 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) { tryReadLong(nbNodes, file, "%s: Cannot read number of nodes: %s", fileName); nodes[fileID].resize(nbNodes); verbosePrint("Reading %u nodes...\n", nbNodes); - for (uint32_t i = nbNodes; i--;) + for (uint32_t i = nbNodes; i--;) { readFileStackNode(file, nodes[fileID], i, fileName); + } // This file's symbols, kept to link sections to them std::vector &fileSymbols = symbolLists.emplace_front(nbSymbols); @@ -553,8 +567,9 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) { readSymbol(file, symbol, fileName, nodes[fileID]); sym_AddSymbol(symbol); - if (symbol.data.holds