Significantly overhaul OPT code

Simplify the mess that was option setting (2 redundant variables !?)
Move options to a separate file
Have "modules" own their options, and OPT only access them (less redundancy)
Simplify code, respect naming conventions better

Makefile

@@ -61,6 +61,7 @@ rgbasm_obj := \
 	src/asm/main.o \
 	src/asm/math.o \
 	src/asm/parser.o \
+	src/asm/opt.o \
 	src/asm/output.o \
 	src/asm/rpn.o \
 	src/asm/section.o \

include/asm/lexer.h

@@ -30,17 +30,21 @@ static inline void lexer_SetStateAtEOL(struct LexerState *state)
 	lexerStateEOL = state;
 }
 
-extern char const *binDigits;
-extern char const *gfxDigits;
+extern char binDigits[2];
+extern char gfxDigits[4];
 
-static inline void lexer_SetBinDigits(char const *digits)
+static inline void lexer_SetBinDigits(char const digits[2])
 {
-	binDigits = digits;
+	binDigits[0] = digits[0];
+	binDigits[1] = digits[1];
 }
 
-static inline void lexer_SetGfxDigits(char const *digits)
+static inline void lexer_SetGfxDigits(char const digits[4])
 {
-	gfxDigits = digits;
+	gfxDigits[0] = digits[0];
+	gfxDigits[1] = digits[1];
+	gfxDigits[2] = digits[2];
+	gfxDigits[3] = digits[3];
 }
 
 /*

include/asm/main.h

@@ -1,7 +1,7 @@
 /*
  * This file is part of RGBDS.
  *
- * Copyright (c) 1997-2018, Carsten Sorensen and RGBDS contributors.
+ * Copyright (c) 1997-2021, Carsten Sorensen and RGBDS contributors.
  *
  * SPDX-License-Identifier: MIT
  */
@@ -15,19 +15,6 @@
 
 #include "helpers.h"
 
-struct sOptions {
-	char binary[2];
-	char gbgfx[4];
-	int32_t fillchar;
-};
-
-extern char *tzNewMacro;
-extern uint32_t ulNewMacroSize;
-extern int32_t nGBGfxID;
-extern int32_t nBinaryID;
-
-extern struct sOptions DefaultOptions;
-extern struct sOptions CurrentOptions;
 extern bool haltnop;
 extern bool optimizeloads;
 extern bool verbose;
@@ -39,10 +26,6 @@ extern bool oGeneratedMissingIncludes;
 extern bool oFailedOnMissingInclude;
 extern bool oGeneratePhonyDeps;
 
-void opt_Push(void);
-void opt_Pop(void);
-void opt_Parse(char *s);
-
 /* TODO: are these really needed? */
 #define YY_FATAL_ERROR fatalerror
 

include/asm/opt.h

@@ -0,0 +1,22 @@
+/*
+ * This file is part of RGBDS.
+ *
+ * Copyright (c) 2021, Eldred Habert and RGBDS contributors.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#ifndef RGBDS_OPT_H
+#define RGBDS_OPT_H
+
+
+void opt_B(char chars[2]);
+void opt_G(char chars[4]);
+void opt_P(uint8_t fill);
+void opt_Parse(char const *option);
+
+void opt_Push(void);
+void opt_Pop(void);
+
+
+#endif

include/asm/section.h

@@ -14,6 +14,8 @@
 
 #include "linkdefs.h"
 
+extern uint8_t fillByte;
+
 struct Expression;
 
 struct Section {

src/asm/lexer.c

@@ -1150,7 +1150,7 @@ static void readFractionalPart(void)
 	yylval.nConstValue |= fractional * (yylval.nConstValue >= 0 ? 1 : -1);
 }
 
-char const *binDigits;
+char binDigits[2];
 
 static void readBinaryNumber(void)
 {
@@ -1210,7 +1210,7 @@ static void readHexNumber(void)
 	yylval.nConstValue = value;
 }
 
-char const *gfxDigits;
+char gfxDigits[4];
 
 static void readGfxConstant(void)
 {

src/asm/main.c

@@ -23,6 +23,7 @@
 #include "asm/fstack.h"
 #include "asm/lexer.h"
 #include "asm/main.h"
+#include "asm/opt.h"
 #include "asm/output.h"
 #include "asm/rpn.h"
 #include "asm/symbol.h"
@@ -55,105 +56,11 @@ bool oFailedOnMissingInclude;
 bool oGeneratePhonyDeps;
 char *tzTargetFileName;
 
-/*
- * Option stack
- */
-
-struct sOptions DefaultOptions;
-struct sOptions CurrentOptions;
 bool haltnop;
 bool optimizeloads;
 bool verbose;
 bool warnings; /* True to enable warnings, false to disable them. */
 
-struct sOptionStackEntry {
-	struct sOptions Options;
-	struct sOptionStackEntry *next;
-};
-
-struct sOptionStackEntry *pOptionStack;
-
-void opt_SetCurrentOptions(struct sOptions *opt)
-{
-	CurrentOptions = *opt;
-	lexer_SetGfxDigits(CurrentOptions.gbgfx);
-	lexer_SetBinDigits(CurrentOptions.binary);
-}
-
-void opt_Parse(char *s)
-{
-	struct sOptions newopt;
-
-	newopt = CurrentOptions;
-
-	switch (s[0]) {
-	case 'g':
-		if (strlen(&s[1]) == 4) {
-			newopt.gbgfx[0] = s[1];
-			newopt.gbgfx[1] = s[2];
-			newopt.gbgfx[2] = s[3];
-			newopt.gbgfx[3] = s[4];
-		} else {
-			error("Must specify exactly 4 characters for option 'g'\n");
-		}
-		break;
-	case 'b':
-		if (strlen(&s[1]) == 2) {
-			newopt.binary[0] = s[1];
-			newopt.binary[1] = s[2];
-		} else {
-			error("Must specify exactly 2 characters for option 'b'\n");
-		}
-		break;
-	case 'p':
-		if (strlen(&s[1]) <= 2) {
-			int result;
-			unsigned int fillchar;
-
-			result = sscanf(&s[1], "%x", &fillchar);
-			if (result != EOF && result != 1)
-				error("Invalid argument for option 'z'\n");
-			else
-				newopt.fillchar = fillchar;
-		} else {
-			error("Invalid argument for option 'z'\n");
-		}
-		break;
-	default:
-		error("Unknown option\n");
-		break;
-	}
-
-	opt_SetCurrentOptions(&newopt);
-}
-
-void opt_Push(void)
-{
-	struct sOptionStackEntry *pOpt;
-
-	pOpt = malloc(sizeof(struct sOptionStackEntry));
-
-	if (pOpt == NULL)
-		fatalerror("No memory for option stack\n");
-
-	pOpt->Options = CurrentOptions;
-	pOpt->next = pOptionStack;
-	pOptionStack = pOpt;
-}
-
-void opt_Pop(void)
-{
-	if (pOptionStack == NULL)
-		fatalerror("No entries in the option stack\n");
-
-	struct sOptionStackEntry *pOpt;
-
-	pOpt = pOptionStack;
-	opt_SetCurrentOptions(&(pOpt->Options));
-	pOptionStack = pOpt->next;
-	free(pOpt);
-}
-
 void opt_AddDefine(char *s)
 {
 	char *value, *equals;
@@ -274,10 +181,8 @@ int main(int argc, char *argv[])
 	int ch;
 	char *ep;
 
-	struct sOptions newopt;
-
 	time_t now = time(NULL);
-	char *sourceDateEpoch = getenv("SOURCE_DATE_EPOCH");
+	char const *sourceDateEpoch = getenv("SOURCE_DATE_EPOCH");
 
 	/*
 	 * Support SOURCE_DATE_EPOCH for reproducible builds
@@ -299,66 +204,60 @@ int main(int argc, char *argv[])
 	yydebug = 1;
 #endif
 
+	// Set defaults
+
 	oGeneratePhonyDeps = false;
 	oGeneratedMissingIncludes = false;
 	oFailedOnMissingInclude = false;
 	tzTargetFileName = NULL;
-	uint32_t maxRecursionDepth = 64;
-	size_t nTargetFileNameLen = 0;
 
-	DefaultOptions.gbgfx[0] = '0';
-	DefaultOptions.gbgfx[1] = '1';
-	DefaultOptions.gbgfx[2] = '2';
-	DefaultOptions.gbgfx[3] = '3';
-	DefaultOptions.binary[0] = '0';
-	DefaultOptions.binary[1] = '1';
-	DefaultOptions.fillchar = 0;
+	opt_B("01");
+	opt_G("0123");
+	opt_P(0);
 	optimizeloads = true;
 	haltnop = true;
 	verbose = false;
 	warnings = true;
+	uint32_t maxRecursionDepth = 64;
+	size_t nTargetFileNameLen = 0;
 	bool exportall = false;
 
-	opt_SetCurrentOptions(&DefaultOptions);
-
-	newopt = CurrentOptions;
-
-	while ((ch = musl_getopt_long_only(argc, argv, optstring, longopts,
-					   NULL)) != -1) {
+	while ((ch = musl_getopt_long_only(argc, argv, optstring, longopts, NULL)) != -1) {
 		switch (ch) {
 		case 'b':
-			if (strlen(optarg) == 2) {
-				newopt.binary[0] = optarg[1];
-				newopt.binary[1] = optarg[2];
-			} else {
+			if (strlen(optarg) == 2)
+				opt_B(&optarg[1]);
+			else
 				errx(1, "Must specify exactly 2 characters for option 'b'");
-			}
 			break;
+
 		case 'D':
 			opt_AddDefine(optarg);
 			break;
+
 		case 'E':
 			exportall = true;
 			break;
+
 		case 'g':
-			if (strlen(optarg) == 4) {
-				newopt.gbgfx[0] = optarg[1];
-				newopt.gbgfx[1] = optarg[2];
-				newopt.gbgfx[2] = optarg[3];
-				newopt.gbgfx[3] = optarg[4];
-			} else {
+			if (strlen(optarg) == 4)
+				opt_G(&optarg[1]);
+			else
 				errx(1, "Must specify exactly 4 characters for option 'g'");
-			}
 			break;
+
 		case 'h':
 			haltnop = false;
 			break;
+
 		case 'i':
 			fstk_AddIncludePath(optarg);
 			break;
+
 		case 'L':
 			optimizeloads = false;
 			break;
+
 		case 'M':
 			if (!strcmp("-", optarg))
 				dependfile = stdout;
@@ -368,34 +267,42 @@ int main(int argc, char *argv[])
 				err(1, "Could not open dependfile %s",
 				    optarg);
 			break;
+
 		case 'o':
 			out_SetFileName(optarg);
 			break;
+
+			unsigned long fill;
 		case 'p':
-			newopt.fillchar = strtoul(optarg, &ep, 0);
+			fill = strtoul(optarg, &ep, 0);
 
 			if (optarg[0] == '\0' || *ep != '\0')
 				errx(1, "Invalid argument for option 'p'");
 
-			if (newopt.fillchar < 0 || newopt.fillchar > 0xFF)
+			if (fill < 0 || fill > 0xFF)
 				errx(1, "Argument for option 'p' must be between 0 and 0xFF");
 
+			opt_P(fill);
 			break;
+
 		case 'r':
 			maxRecursionDepth = strtoul(optarg, &ep, 0);
 
 			if (optarg[0] == '\0' || *ep != '\0')
 				errx(1, "Invalid argument for option 'r'");
 			break;
+
 		case 'V':
 			printf("rgbasm %s\n", get_package_version_string());
 			exit(0);
 		case 'v':
 			verbose = true;
 			break;
+
 		case 'W':
 			processWarningFlag(optarg);
 			break;
+
 		case 'w':
 			warnings = false;
 			break;
@@ -406,9 +313,11 @@ int main(int argc, char *argv[])
 			case 'G':
 				oGeneratedMissingIncludes = true;
 				break;
+
 			case 'P':
 				oGeneratePhonyDeps = true;
 				break;
+
 			case 'Q':
 			case 'T':
 				if (optind == argc)
@@ -453,10 +362,6 @@ int main(int argc, char *argv[])
 	if (tzTargetFileName == NULL)
 		tzTargetFileName = tzObjectname;
 
-	opt_SetCurrentOptions(&newopt);
-
-	DefaultOptions = CurrentOptions;
-
 	if (argc == optind) {
 		fputs("FATAL: No input files\n", stderr);
 		print_usage();
@@ -487,8 +392,6 @@ int main(int argc, char *argv[])
 	opt_ParseDefines();
 	charmap_New("main", NULL);
 
-	opt_SetCurrentOptions(&DefaultOptions);
-
 	if (yyparse() != 0 || nbErrors != 0)
 		errx(1, "Assembly aborted (%u errors)!", nbErrors);
 	if (dependfile)

src/asm/opt.c

@@ -0,0 +1,111 @@
+
+#include <errno.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "asm/lexer.h"
+#include "asm/section.h"
+#include "asm/warning.h"
+
+struct OptStackEntry {
+	char binary[2];
+	char gbgfx[4];
+	int32_t fillByte;
+	struct OptStackEntry *next;
+};
+
+static struct OptStackEntry *stack = NULL;
+
+void opt_B(char chars[2])
+{
+	lexer_SetBinDigits(chars);
+}
+
+void opt_G(char chars[4])
+{
+	lexer_SetGfxDigits(chars);
+}
+
+void opt_P(uint8_t fill)
+{
+	fillByte = fill;
+}
+
+void opt_Parse(char *s)
+{
+	switch (s[0]) {
+	case 'b':
+		if (strlen(&s[1]) == 2)
+			opt_B(&s[1]);
+		else
+			error("Must specify exactly 2 characters for option 'b'\n");
+		break;
+
+	case 'g':
+		if (strlen(&s[1]) == 4)
+			opt_G(&s[1]);
+		else
+			error("Must specify exactly 4 characters for option 'g'\n");
+		break;
+
+	case 'p':
+		if (strlen(&s[1]) <= 2) {
+			int result;
+			unsigned int fillchar;
+
+			result = sscanf(&s[1], "%x", &fillchar);
+			if (result != EOF && result != 1)
+				error("Invalid argument for option 'p'\n");
+			else
+				opt_P(fillchar);
+		} else {
+			error("Invalid argument for option 'p'\n");
+		}
+		break;
+
+	default:
+		error("Unknown option '%c'\n", s[0]);
+		break;
+	}
+}
+
+void opt_Push(void)
+{
+	struct OptStackEntry *entry = malloc(sizeof(*entry));
+
+	if (entry == NULL)
+		fatalerror("Failed to alloc option stack entry: %s\n", strerror(errno));
+
+	// Both of these pulled from lexer.h
+	entry->binary[0] = binDigits[0];
+	entry->binary[1] = binDigits[1];
+
+	entry->gbgfx[0] = gfxDigits[0];
+	entry->gbgfx[1] = gfxDigits[1];
+	entry->gbgfx[2] = gfxDigits[2];
+	entry->gbgfx[3] = gfxDigits[3];
+
+	entry->fillByte = fillByte; // Pulled from section.h
+
+	entry->next = stack;
+	stack = entry;
+}
+
+void opt_Pop(void)
+{
+	if (stack == NULL) {
+		error("No entries in the option stack\n");
+		return;
+	}
+
+	struct OptStackEntry *entry = stack;
+
+	opt_B(entry->binary);
+	opt_G(entry->gbgfx);
+	opt_P(entry->fillByte);
+	stack = entry->next;
+	free(entry);
+}

src/asm/parser.y

@@ -24,6 +24,7 @@
 #include "asm/macro.h"
 #include "asm/main.h"
 #include "asm/mymath.h"
+#include "asm/opt.h"
 #include "asm/output.h"
 #include "asm/rpn.h"
 #include "asm/section.h"

src/asm/section.c

@@ -16,6 +16,8 @@
 #include "extern/err.h"
 #include "platform.h" // strdup
 
+uint8_t fillByte;
+
 struct SectionStackEntry {
 	struct Section *section;
 	char const *scope; /* Section's symbol scope */
@@ -543,7 +545,7 @@ void out_Skip(int32_t skip, bool ds)
 	} else {
 		checkcodesection();
 		while (skip--)
-			writebyte(CurrentOptions.fillchar);
+			writebyte(fillByte);
 	}
 }