Use std::vector for reading object file symbols

include/link/patch.hpp

@@ -6,6 +6,7 @@
 
 #include <deque>
 #include <stdint.h>
+#include <vector>
 
 #include "link/section.hpp"
 
@@ -15,7 +16,7 @@ struct Assertion {
 	struct Patch patch; // Also used for its `.type`
 	char *message;
 	// This would be redundant with `.section->fileSymbols`... but `section` is sometimes NULL!
-	struct Symbol **fileSymbols;
+	std::vector<struct Symbol *> *fileSymbols;
 };
 
 /*

include/link/sdas_obj.hpp

@@ -5,9 +5,11 @@
 #define RGBDS_LINK_SDAS_OBJ_H
 
 #include <stdio.h>
+#include <vector>
 
 struct FileStackNode;
+struct Symbol;
 
-void sdobj_ReadFile(struct FileStackNode const *fileName, FILE *file);
+void sdobj_ReadFile(struct FileStackNode const *fileName, FILE *file, std::vector<struct Symbol *> &fileSymbols);
 
 #endif // RGBDS_LINK_SDAS_OBJ_H

include/link/section.hpp

@@ -47,7 +47,7 @@ struct Section {
 	uint8_t *data; // Array of size `size`
 	std::vector<struct Patch> *patches;
 	// Extra info computed during linking
-	struct Symbol **fileSymbols;
+	std::vector<struct Symbol *> *fileSymbols;
 	uint32_t nbSymbols;
 	struct Symbol **symbols;
 	struct Section *nextu; // The next "component" of this unionized sect

src/link/object.cpp

@@ -24,13 +24,7 @@
 #include "linkdefs.hpp"
 #include "version.hpp"
 
-struct SymbolList {
-	size_t nbSymbols;
-	struct Symbol **symbolList;
-};
-
-static std::deque<struct SymbolList> symbolLists;
-
+static std::deque<std::vector<struct Symbol *>> symbolLists;
 static std::vector<std::vector<struct FileStackNode>> nodes;
 static std::deque<struct Assertion> assertions;
 
@@ -478,7 +472,9 @@ void obj_ReadFile(char const *fileName, unsigned int fileID)
 		if (!where.name)
 			fatal(NULL, 0, "Failed to duplicate \"%s\"'s name: %s", fileName, strerror(errno));
 
-		sdobj_ReadFile(&where, file);
+		std::vector<struct Symbol *> &fileSymbols = symbolLists.emplace_front();
+
+		sdobj_ReadFile(&where, file, fileSymbols);
 		return;
 	}
 
@@ -516,13 +512,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID)
 		readFileStackNode(file, nodes[fileID], i, fileName);
 
 	// This file's symbols, kept to link sections to them
-	struct Symbol **fileSymbols = (struct Symbol **)malloc(sizeof(*fileSymbols) * nbSymbols + 1);
-
-	if (!fileSymbols)
-		err("Failed to get memory for %s's symbols", fileName);
-
-	symbolLists.push_front({ .nbSymbols = nbSymbols, .symbolList = fileSymbols });
-
+	std::vector<struct Symbol *> &fileSymbols = symbolLists.emplace_front(nbSymbols);
 	std::vector<uint32_t> nbSymPerSect(nbSections, 0);
 
 	verbosePrint("Reading %" PRIu32 " symbols...\n", nbSymbols);
@@ -553,7 +543,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID)
 
 		fileSections[i]->nextu = NULL;
 		readSection(file, fileSections[i], fileName, nodes[fileID]);
-		fileSections[i]->fileSymbols = fileSymbols;
+		fileSections[i]->fileSymbols = &fileSymbols;
 		if (nbSymPerSect[i]) {
 			fileSections[i]->symbols =
 				(struct Symbol **)malloc(nbSymPerSect[i] * sizeof(*fileSections[i]->symbols));
@@ -606,7 +596,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID)
 
 		readAssertion(file, &assertion, fileName, i, nodes[fileID]);
 		linkPatchToPCSect(&assertion.patch, fileSections);
-		assertion.fileSymbols = fileSymbols;
+		assertion.fileSymbols = &fileSymbols;
 	}
 
 	fclose(file);
@@ -670,10 +660,9 @@ void obj_Cleanup(void)
 	sect_ForEach(freeSection);
 	sect_CleanupSections();
 
-	for (struct SymbolList &list : symbolLists) {
-		for (size_t i = 0; i < list.nbSymbols; i++)
-			freeSymbol(list.symbolList[i]);
-		free(list.symbolList);
+	for (std::vector<struct Symbol *> &fileSymbols : symbolLists) {
+		for (struct Symbol *symbol : fileSymbols)
+			freeSymbol(symbol);
 	}
 
 	for (struct Assertion &assert : assertions)

src/link/patch.cpp

@@ -56,11 +56,10 @@ static uint32_t getRPNByte(uint8_t const **expression, int32_t *size,
 	return *(*expression)++;
 }
 
-static struct Symbol const *getSymbol(struct Symbol const * const *symbolList,
-				      uint32_t index)
+static struct Symbol const *getSymbol(std::vector<struct Symbol *> const &symbolList, uint32_t index)
 {
 	assert(index != (uint32_t)-1); // PC needs to be handled specially, not here
-	struct Symbol const *symbol = symbolList[index];
+	struct Symbol *symbol = symbolList[index];
 
 	// If the symbol is defined elsewhere...
 	if (symbol->type == SYMTYPE_IMPORT)
@@ -78,7 +77,7 @@ static struct Symbol const *getSymbol(struct Symbol const * const *symbolList,
  *                 errors caused by the value should be suppressed.
  */
 static int32_t computeRPNExpr(struct Patch const *patch,
-			      struct Symbol const * const *fileSymbols)
+			      std::vector<struct Symbol *> const &fileSymbols)
 {
 // Small shortcut to avoid a lot of repetition
 #define popRPN() popRPN(patch->src, patch->lineNo)
@@ -416,8 +415,7 @@ void patch_CheckAssertions(std::deque<struct Assertion> &assertions)
 	verbosePrint("Checking assertions...\n");
 
 	for (struct Assertion &assert : assertions) {
-		int32_t value = computeRPNExpr(&assert.patch,
-			(struct Symbol const * const *)assert.fileSymbols);
+		int32_t value = computeRPNExpr(&assert.patch, *assert.fileSymbols);
 		enum AssertionType type = (enum AssertionType)assert.patch.type;
 
 		if (!isError && !value) {
@@ -455,9 +453,7 @@ static void applyFilePatches(struct Section *section, struct Section *dataSectio
 {
 	verbosePrint("Patching section \"%s\"...\n", section->name);
 	for (struct Patch &patch : *section->patches) {
-		int32_t value = computeRPNExpr(&patch,
-					       (struct Symbol const * const *)
-							section->fileSymbols);
+		int32_t value = computeRPNExpr(&patch, *section->fileSymbols);
 		uint16_t offset = patch.offset + section->offset;
 
 		// `jr` is quite unlike the others...

src/link/sdas_obj.cpp

@@ -144,7 +144,7 @@ enum RelocFlags {
 		| 1 << RELOC_EXPR24 | 1 << RELOC_BANKBYTE,
 };
 
-void sdobj_ReadFile(struct FileStackNode const *where, FILE *file) {
+void sdobj_ReadFile(struct FileStackNode const *where, FILE *file, std::vector<struct Symbol *> &fileSymbols) {
 	size_t bufLen = 256;
 	char *line = (char *)malloc(bufLen);
 	char const *token;
@@ -234,11 +234,7 @@ void sdobj_ReadFile(struct FileStackNode const *where, FILE *file) {
 		uint16_t writeIndex;
 	};
 	std::vector<struct FileSection> fileSections;
-	struct Symbol **fileSymbols = (struct Symbol **)malloc(sizeof(*fileSymbols) * expectedNbSymbols);
 	size_t nbSections = 0, nbSymbols = 0;
-
-	if (!fileSymbols)
-		fatal(where, lineNo, "Failed to alloc file symbols table: %s", strerror(errno));
 	size_t nbBytes = 0; // How many bytes are in `data`, including the ADDR_SIZE "header" bytes
 	size_t dataCapacity = 16 + ADDR_SIZE; // SDCC object files usually contain 16 bytes per T line
 	uint8_t *data = (uint8_t *)malloc(sizeof(*data) * dataCapacity);
@@ -262,10 +258,11 @@ void sdobj_ReadFile(struct FileStackNode const *where, FILE *file) {
 					expectedNbAreas);
 			fileSections.resize(nbSections + 1);
 			fileSections[nbSections].writeIndex = 0;
-#define curSection (fileSections[nbSections].section)
-			curSection = (struct Section *)malloc(sizeof(*curSection));
+			struct Section *curSection = (struct Section *)malloc(sizeof(*curSection));
+
 			if (!curSection)
 				fatal(where, lineNo, "Failed to alloc new area: %s", strerror(errno));
+			fileSections[nbSections].section = curSection;
 
 			getToken(line, "'A' line is too short");
 			assert(strlen(token) != 0); // This should be impossible, tokens are non-empty
@@ -357,36 +354,24 @@ void sdobj_ReadFile(struct FileStackNode const *where, FILE *file) {
 			if (!curSection->patches)
 				fatal(where, lineNo, "Failed to alloc new area's patches: %s",
 				      strerror(errno));
-			curSection->fileSymbols = fileSymbols; // IDs are instead per-section
+			curSection->fileSymbols = &fileSymbols; // IDs are instead per-section
 			curSection->nbSymbols = 0;
 			curSection->symbols = NULL; // Will be allocated on demand as well
 			curSection->nextu = NULL;
-#undef curSection
+
 			++nbSections;
 			break;
 		}
 
-		case 'S':
+		case 'S': {
 			if (nbSymbols == expectedNbSymbols)
 				warning(where, lineNo, "Got more 'S' lines than the expected %" PRIu32,
 					expectedNbSymbols);
-			// `realloc` is dangerous, as sections contain a pointer to `fileSymbols`.
-			// We can try to be nice, but if the pointer moves, it's game over!
-			if (nbSymbols >= expectedNbSymbols) {
-				struct Symbol **newFileSymbols = (struct Symbol **)realloc(fileSymbols,
-					sizeof(*fileSymbols) * (nbSymbols + 1));
+			struct Symbol *symbol = (struct Symbol *)malloc(sizeof(*symbol));
 
-				if (!newFileSymbols)
-					fatal(where, lineNo, "Failed to alloc extra symbols: %s",
-					      strerror(errno));
-				if (newFileSymbols != fileSymbols)
-					fatal(where, lineNo, "Failed to handle extra 'S' lines (pointer moved)");
-				// No need to assign, obviously
-			}
-#define symbol (fileSymbols[nbSymbols])
-			symbol = (struct Symbol *)malloc(sizeof(*symbol));
 			if (!symbol)
 				fatal(where, lineNo, "Failed to alloc symbol: %s", strerror(errno));
+			fileSymbols.push_back(symbol);
 
 			// Init other members
 			symbol->objFileName = where->name;
@@ -453,12 +438,12 @@ void sdobj_ReadFile(struct FileStackNode const *where, FILE *file) {
 					      section->name, strerror(errno));
 				section->symbols[section->nbSymbols - 1] = symbol;
 			}
-#undef symbol
 
 			expectEol("'S' line is too long");
 
 			++nbSymbols;
 			break;
+		}
 
 		case 'T':
 			// Now, time to parse the data!