From 8a49a0b71456119d5d444b26db4927db405e417c Mon Sep 17 00:00:00 2001 From: Sylvie <35663410+Rangi42@users.noreply.github.com> Date: Sun, 18 Feb 2024 11:07:25 -0500 Subject: [PATCH] Report "" or "" when using "-" as a filename placeholder (#1297) Also fix a memory leak with `targetFileNames` --- include/asm/output.hpp | 2 +- include/link/main.hpp | 14 ------------- src/asm/main.cpp | 12 +++++------ src/asm/output.cpp | 14 +++++++------ src/link/main.cpp | 19 ----------------- src/link/object.cpp | 24 +++++++++------------ src/link/output.cpp | 47 +++++++++++++++++++++++++++++++++++------- 7 files changed, 64 insertions(+), 68 deletions(-) diff --git a/include/asm/output.hpp b/include/asm/output.hpp index 79e966ba..323c6969 100644 --- a/include/asm/output.hpp +++ b/include/asm/output.hpp @@ -10,7 +10,7 @@ struct Expression; struct FileStackNode; -extern char *objectName; +extern const char *objectName; extern struct Section *sectionList; void out_RegisterNode(struct FileStackNode *node); diff --git a/include/link/main.hpp b/include/link/main.hpp index 80aa2567..b2beb4b0 100644 --- a/include/link/main.hpp +++ b/include/link/main.hpp @@ -64,18 +64,4 @@ void error(struct FileStackNode const *where, uint32_t lineNo, [[noreturn]] void fatal(struct FileStackNode const *where, uint32_t lineNo, char const *fmt, ...) format_(printf, 3, 4); -/* - * Opens a file if specified, and aborts on error. - * @param fileName The name of the file to open; if NULL, no file will be opened - * @param mode The mode to open the file with - * @return A pointer to a valid FILE structure, or NULL if fileName was NULL - */ -FILE *openFile(char const *fileName, char const *mode); - -#define closeFile(file) do { \ - FILE *tmp = file; \ - if (tmp) \ - fclose(tmp); \ - } while (0) - #endif // RGBDS_LINK_MAIN_H diff --git a/src/asm/main.cpp b/src/asm/main.cpp index 78952ec3..f28e4bb0 100644 --- a/src/asm/main.cpp +++ b/src/asm/main.cpp @@ -259,7 +259,7 @@ int main(int argc, char *argv[]) dependFileName = musl_optarg; } if (dependfile == NULL) - err("Failed to open dependfile %s", dependFileName); + err("Failed to open dependfile \"%s\"", dependFileName); break; case 'o': @@ -362,8 +362,9 @@ int main(int argc, char *argv[]) memcpy(&targetFileName[targetFileNameLen], newTarget, newTargetLen); if (depType == 'Q') free(newTarget); + if (targetFileNameLen > 0) + targetFileName[targetFileNameLen - 1] = ' '; targetFileNameLen += newTargetLen; - targetFileName[targetFileNameLen - 1] = ' '; break; } break; @@ -376,10 +377,8 @@ int main(int argc, char *argv[]) } } - if (targetFileName == NULL) - targetFileName = objectName; - else - targetFileName[targetFileNameLen - 1] = '\0'; // Overwrite the last space + if (!targetFileName && objectName) + targetFileName = strdup(objectName); if (argc == musl_optind) { fputs("FATAL: Please specify an input file (pass `-` to read from standard input)\n", stderr); @@ -415,6 +414,7 @@ int main(int argc, char *argv[]) if (dependfile) fclose(dependfile); + free(targetFileName); sect_CheckUnionClosed(); diff --git a/src/asm/output.cpp b/src/asm/output.cpp index 1cbfeea0..838e4ad2 100644 --- a/src/asm/output.cpp +++ b/src/asm/output.cpp @@ -42,7 +42,7 @@ struct Assertion { struct Assertion *next; }; -char *objectName; +const char *objectName; struct Section *sectionList; @@ -480,13 +480,15 @@ static void registerUnregisteredSymbol(struct Symbol *symbol, void *) void out_WriteObject(void) { FILE *f; - if (strcmp(objectName, "-") != 0) - f = fopen(objectName, "wb"); - else - f = fdopen(STDOUT_FILENO, "wb"); + if (strcmp(objectName, "-")) { + f = fopen(objectName, "wb"); + } else { + objectName = ""; + f = fdopen(STDOUT_FILENO, "wb"); + } if (!f) - err("Couldn't write file '%s'", objectName); + err("Failed to open object file '%s'", objectName); // Also write symbols that weren't written above sym_ForEach(registerUnregisteredSymbol, NULL); diff --git a/src/link/main.cpp b/src/link/main.cpp index e4315e43..552a88f8 100644 --- a/src/link/main.cpp +++ b/src/link/main.cpp @@ -142,25 +142,6 @@ void argErr(char flag, char const *fmt, ...) exit(1); } -FILE *openFile(char const *fileName, char const *mode) -{ - if (!fileName) - return NULL; - - FILE *file; - if (strcmp(fileName, "-") != 0) - file = fopen(fileName, mode); - else if (mode[0] == 'r') - file = fdopen(STDIN_FILENO, mode); - else - file = fdopen(STDOUT_FILENO, mode); - - if (!file) - err("Failed to open file \"%s\"", fileName); - - return file; -} - // Short options static const char *optstring = "dl:m:Mn:O:o:p:S:s:tVvWwx"; diff --git a/src/link/object.cpp b/src/link/object.cpp index 8867018a..cf50447d 100644 --- a/src/link/object.cpp +++ b/src/link/object.cpp @@ -446,13 +446,14 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) { FILE *file; - if (strcmp("-", fileName) != 0) + if (strcmp(fileName, "-")) { file = fopen(fileName, "rb"); - else + } else { + fileName = ""; file = fdopen(STDIN_FILENO, "rb"); // `stdin` is in text mode by default - + } if (!file) - err("Failed to open file %s", fileName); + err("Failed to open file \"%s\"", fileName); // First, check if the object is a RGBDS object or a SDCC one. If the first byte is 'R', // we'll assume it's a RGBDS object file, and otherwise, that it's a SDCC object file. @@ -495,13 +496,11 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) && matchedElems != strlen(RGBDS_OBJECT_VERSION_STRING)) errx("%s: Not a RGBDS object file", fileName); - verbosePrint("Reading object file %s\n", - fileName); + verbosePrint("Reading object file %s\n", fileName); uint32_t revNum; - tryReadlong(revNum, file, "%s: Cannot read revision number: %s", - fileName); + tryReadlong(revNum, file, "%s: Cannot read revision number: %s", fileName); if (revNum != RGBDS_OBJECT_REV) errx("%s: Unsupported object file for rgblink %s; try rebuilding \"%s\"%s" " (expected revision %d, got %d)", fileName, get_package_version_string(), @@ -511,10 +510,8 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) uint32_t nbSymbols; uint32_t nbSections; - tryReadlong(nbSymbols, file, "%s: Cannot read number of symbols: %s", - fileName); - tryReadlong(nbSections, file, "%s: Cannot read number of sections: %s", - fileName); + tryReadlong(nbSymbols, file, "%s: Cannot read number of symbols: %s", fileName); + tryReadlong(nbSections, file, "%s: Cannot read number of sections: %s", fileName); nbSectionsToAssign += nbSections; @@ -622,8 +619,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) uint32_t nbAsserts; - tryReadlong(nbAsserts, file, "%s: Cannot read number of assertions: %s", - fileName); + tryReadlong(nbAsserts, file, "%s: Cannot read number of assertions: %s", fileName); verbosePrint("Reading %" PRIu32 " assertions...\n", nbAsserts); for (uint32_t i = 0; i < nbAsserts; i++) { struct Assertion *assertion = (struct Assertion *)malloc(sizeof(*assertion)); diff --git a/src/link/output.cpp b/src/link/output.cpp index c6f94b52..8e59ae7f 100644 --- a/src/link/output.cpp +++ b/src/link/output.cpp @@ -231,8 +231,27 @@ static void writeBank(struct SortedSection *bankSections, uint16_t baseOffset, // Writes a ROM file to the output. static void writeROM(void) { - outputFile = openFile(outputFileName, "wb"); - overlayFile = openFile(overlayFileName, "rb"); + if (outputFileName) { + if (strcmp(outputFileName, "-")) { + outputFile = fopen(outputFileName, "wb"); + } else { + outputFileName = ""; + outputFile = fdopen(STDOUT_FILENO, "wb"); + } + if (!outputFile) + err("Failed to open output file \"%s\"", outputFileName); + } + + if (overlayFile) { + if (strcmp(overlayFileName, "-")) { + overlayFile = fopen(overlayFileName, "rb"); + } else { + overlayFileName = ""; + overlayFile = fdopen(STDIN_FILENO, "rb"); + } + if (!overlayFile) + err("Failed to open overlay file \"%s\"", overlayFileName); + } uint32_t nbOverlayBanks = checkOverlaySize(); @@ -249,8 +268,10 @@ static void writeROM(void) sectionTypeInfo[SECTTYPE_ROMX].startAddr, sectionTypeInfo[SECTTYPE_ROMX].size); } - closeFile(outputFile); - closeFile(overlayFile); + if (outputFile) + fclose(outputFile); + if (overlayFile) + fclose(overlayFile); } /* @@ -555,7 +576,12 @@ static void writeSym(void) if (!symFileName) return; - symFile = openFile(symFileName, "w"); + if (strcmp(symFileName, "-")) { + symFile = fopen(symFileName, "w"); + } else { + symFileName = ""; + symFile = fdopen(STDOUT_FILENO, "w"); + } if (!symFile) err("Failed to open sym file \"%s\"", symFileName); @@ -568,7 +594,7 @@ static void writeSym(void) writeSymBank(§ions[type].banks[bank], type, bank); } - closeFile(symFile); + fclose(symFile); } // Writes the map file, if applicable. @@ -577,7 +603,12 @@ static void writeMap(void) if (!mapFileName) return; - mapFile = openFile(mapFileName, "w"); + if (strcmp(mapFileName, "-")) { + mapFile = fopen(mapFileName, "w"); + } else { + mapFileName = ""; + mapFile = fdopen(STDOUT_FILENO, "w"); + } if (!mapFile) err("Failed to open map file \"%s\"", mapFileName); @@ -590,7 +621,7 @@ static void writeMap(void) writeMapBank(§ions[type].banks[bank], type, bank); } - closeFile(mapFile); + fclose(mapFile); } static void cleanupSections(struct SortedSection *section)