From 964c08a6a36085868f26710a80c056467a341224 Mon Sep 17 00:00:00 2001
From: Jeff Quast <contact@jeffquast.com>
Date: Mon, 24 Jul 2023 01:33:48 -0400
Subject: [PATCH] - Fix dos33.c getopt for BSAVE arguments    
 https://github.com/deater/dos33fsprogs/issues/18

- tested on OSX and Debian arm64
- bugfix display of length when `-d` debug is used
- small fixes to usage terms
---
 utils/dos33fs-utils/dos33.c                | 102 ++++++++++++++++-----
 utils/dos33fs-utils/tests/run_all_tests.sh |   1 +
 utils/dos33fs-utils/tests/test_bsave.sh    |  36 ++++++++
 3 files changed, 116 insertions(+), 23 deletions(-)
 create mode 100755 utils/dos33fs-utils/tests/test_bsave.sh

diff --git a/utils/dos33fs-utils/dos33.c b/utils/dos33fs-utils/dos33.c
index 79ff1434b..82c85c8e8 100644
--- a/utils/dos33fs-utils/dos33.c
+++ b/utils/dos33fs-utils/dos33.c
@@ -955,9 +955,9 @@ static void display_help(char *name, int version_only) {
 	printf("  Where disk_image is a valid dos3.3 disk image\n"
 		"  and COMMAND is one of the following:\n");
 	printf("\tCATALOG\n");
-	printf("\tLOAD     apple_file <local_file>\n");
-	printf("\tSAVE     type local_file <apple_file>\n");
-	printf("\tBSAVE    [-a addr] [-l len] local_file <apple_file>\n");
+	printf("\tLOAD     apple_file [local_file]\n");
+	printf("\tSAVE     type local_file [apple_file]\n");
+	printf("\tBSAVE    [-a addr] [-l len] local_file [apple_file]\n");
 	printf("\tDELETE   apple_file\n");
 	printf("\tLOCK     apple_file\n");
 	printf("\tUNLOCK   apple_file\n");
@@ -1058,25 +1058,31 @@ int main(int argc, char **argv) {
 	char *temp,*endptr;
 	int c;
 	int address=0, length=0;
+	int swp_optind=0;
 	unsigned char vtoc[BYTES_PER_SECTOR];
 	int retval=0;
 
 	/* Check command line arguments */
-	while ((c = getopt (argc, argv,"a:l:t:s:dhvxy"))!=-1) {
-		switch (c) {
+	while ((c = getopt(argc, argv, "a:l:t:s:dhvxy")) != -1)
+		{
+			switch (c)
+			{
+
+			case 'd':
+				fprintf(stderr, "DEBUG enabled\n");
+				debug = 1;
+				break;
+			case 'l':
+				length = strtol(optarg, &endptr, 0);
+				if (debug)
+					fprintf(stderr, "Length=%d\n", length);
+				break;
+			case 'a':
+				address = strtol(optarg, &endptr, 0);
+				if (debug)
+					fprintf(stderr, "Address=%d\n", address);
+				break;
 
-		case 'd':
-			fprintf(stderr,"DEBUG enabled\n");
-			debug=1;
-			break;
-		case 'a':
-			address=strtol(optarg,&endptr,0);
-			if (debug) fprintf(stderr,"Address=%d\n",address);
-			break;
-		case 'l':
-			length=strtol(optarg,&endptr,0);
-			if (debug) fprintf(stderr,"Length=%d\n",address);
-			break;
 #if 0
 		case 't':
 			track=strtol(optarg,&endptr,0);
@@ -1100,8 +1106,11 @@ int main(int argc, char **argv) {
 			break;
 		}
 	}
-
-	if (optind==argc) {
+	for (c = 0; c < argc;c++) {
+		printf("+ argv[%d]=%s\n",c,argv[c]);
+	}
+	if (optind == argc)
+	{
 		fprintf(stderr,"ERROR!  Must specify disk image!\n\n");
 		return -ERROR_INVALID_PARAMATER;
 	}
@@ -1124,6 +1133,7 @@ int main(int argc, char **argv) {
 		retval=-ERROR_INVALID_PARAMATER;
 		goto exit_and_close;
 	}
+	printf("after next argument, optind=%d (%s) (command)\n", optind, argv[optind]);
 
 	/* Grab command */
 	strncpy(temp_string,argv[optind],BUFSIZ-1);
@@ -1135,6 +1145,7 @@ int main(int argc, char **argv) {
 
 	/* Move to next argument */
 	optind++;
+	printf("after next argument, optind=%d (%s) [...]\n", optind, argv[optind]);
 
 	command=lookup_command(temp_string);
 
@@ -1223,18 +1234,63 @@ int main(int argc, char **argv) {
 
 		if (debug) printf("\ttype=%c\n",type);
 
+		if (command==COMMAND_BSAVE) {
+			// Find position of 'BSAVE' command in argv, and call getopt(3) a
+			// second time to forward position of optind beyond optional -a and
+			// -l arguments on BSD.
+			//
+			// This is necessary because BSD and Linux getopt(3) differs, linux
+			// will process '-a' and '-l' options and mutate argv for the value
+			// of optind to point to remaining arguments, "local_filename //
+			// [apple_file]".
+			//
+			// While BSD does not mutate argv, leaving optind at the position of
+			// the first unknown option, 'BSAVE' with remaining arguments, "[-a
+			// addr] [-l len] local_filename [apple_file]" still remaining for
+			// processing, and this is why we must call getopt(3) a second time,
+			// in such a peculiar way.
+			optind = 1;
+			while ((strncmp(argv[swp_optind], "BSAVE", 5)) && swp_optind < argc)
+			{
+				swp_optind++;
+			}
+			swp_optind++;
+
+			// forward argv and decrement argc past BSAVE command and reset optind
+			argv += (swp_optind - 1);
+			argc -= (swp_optind - 1);
+			optind = 1;
+			while ((c = getopt(argc, argv, "a:l:")) != -1)
+			{
+				switch (c) {
+					case 'h':
+						display_help(argv[0], 0);
+						return 0;
+					case 'l':
+						length = strtol(optarg, &endptr, 0);
+						if (debug)
+							fprintf(stderr, "Length=%d\n", length);
+						break;
+					case 'a':
+						address = strtol(optarg, &endptr, 0);
+						if (debug)
+							fprintf(stderr, "Address=%d\n", address);
+						break;
+				}
+			}
+		}
+
 		if (argc==optind) {
-			fprintf(stderr,"Error! Need file_name\n");
+			fprintf(stderr, "Error! local_file argument required\n");
 
 			if (command==COMMAND_BSAVE) {
 				fprintf(stderr,"%s %s BSAVE "
-						"file_name apple_filename\n\n",
+						"[-a addr] [-l len] local_file [apple_file]\n\n",
 						argv[0],image);
-
 			}
 			else {
 				fprintf(stderr,"%s %s SAVE type "
-						"file_name apple_filename\n\n",
+						"local_file [apple_file]\n\n",
 						argv[0],image);
 			}
 			retval=-ERROR_INVALID_PARAMATER;
diff --git a/utils/dos33fs-utils/tests/run_all_tests.sh b/utils/dos33fs-utils/tests/run_all_tests.sh
index 022082101..09bf91a0d 100755
--- a/utils/dos33fs-utils/tests/run_all_tests.sh
+++ b/utils/dos33fs-utils/tests/run_all_tests.sh
@@ -9,3 +9,4 @@
 ./test.sh
 ./test_undelete2.sh
 ./test_undelete.sh
+./test_bsave.sh
\ No newline at end of file
diff --git a/utils/dos33fs-utils/tests/test_bsave.sh b/utils/dos33fs-utils/tests/test_bsave.sh
new file mode 100755
index 000000000..f01cbc47f
--- /dev/null
+++ b/utils/dos33fs-utils/tests/test_bsave.sh
@@ -0,0 +1,36 @@
+echo "Testing dos33 BSAVE"
+
+fail() {
+    echo "$1" >&2
+    num_failed=$(($num_failed + 1))
+}
+num_failed=0
+
+tempfile=$(mktemp -t temp.XXXXXXXXXX)
+run_test() {
+    cmd_args=$1
+    shift
+    expect_strings="$@"
+    # run and assert return code is zero
+    ../dos33 -d ./test.dsk $cmd_args >"$tempfile" 2>&1 || fail "Command failed: $(cat "$tempfile")"
+    # assert all expected strings are present in output
+    for expect_str in $expect_strings
+    do
+        grep "$expect_str" "$tempfile" >/dev/null || fail "Command output missing expected string, '$expect_str'"
+    done
+}
+
+cp empty.dsk test.dsk
+run_test "BSAVE SINCOS" \
+         "type=b" "Local filename: SINCOS" "Apple filename: SINCOS"
+run_test "BSAVE SINCOS EX1" \
+         "type=b" "Local filename: SINCOS" "Apple filename: EX1"
+run_test "BSAVE -a 0x2000 SINCOS EX2" \
+         "type=b" "Local filename: SINCOS" "Apple filename: EX2" "Address=8192"
+run_test "BSAVE -a 0x2000 -l 146 SINCOS EX3" \
+         "type=b" "Local filename: SINCOS" "Apple filename: EX3" "Address=8192" "Length=146"
+rm "$tempfile"
+if [ "$num_failed" -gt 0 ]; then
+    echo "Test failed. $num_failed tests failed."
+    exit 1
+fi
\ No newline at end of file