Tim's refactor of track0 sectors for os9 format

build/unix/unittest/Makefile

@@ -4,17 +4,18 @@ include ../rules.mak
 vpath %.c ../../../unittest
 
 LDLIBS += -L../libtoolshed -L../libcoco -L../libcecb -L../libdecb \
-          -L../librbf -L../libmisc -L../libnative -L../libsys \
-          -ltoolshed -lcoco -lcecb -ldecb -lrbf -lmisc -lnative -lsys -lm
+          -L../librbf -L../libnative -L../libmisc -L../libsys \
+          -ltoolshed -lcoco -lcecb -ldecb -lrbf -lnative -lmisc -lsys -lm
 
-TESTS = librbftest libdecbtest libcecbtest libtoolshedtest
+TESTS = librbftest libdecbtest libcecbtest libtoolshedtest os9commandtest
 
 testall: $(TESTS)
 ifeq (,$(NOTEST))
 	./librbftest
 	./libdecbtest
 	./libcecbtest
 	./libtoolshedtest
+	./os9commandtest
 endif
 
 clean:

os9/os9format.c

@@ -33,11 +33,11 @@ static char const *const helpMessage[] = {
 	"     -ss  = single sided (default)\n",
 	"     -ds  = double sided\n",
 	"     -tX  = tracks (default = 35)\n",
-	"     -stX = sectors per track (default = 18)\n",	
+	"     -stX = sectors per track (default = 18)\n",
 	"     -szX = sectors for track 0 (default = 18)\n",
 	"     -dr  = format a Dragon disk\n",
 	" Hard Drive Options:\n",
-	"     -lX  = number of logical sectors\n",
+	"     -lX  = number of logical sectors (floppy options ignored)\n",
 	NULL
 };
 
@@ -51,7 +51,7 @@ int os9format(int argc, char **argv)
 	int tracks = 35;
 	int heads = 1;
 	int sectorsPerTrack = DEF_SECTORS_TRACK;
-	int sectorsTrack0 = DEF_SECTORS_TRACK;
+	int sectorsTrack0 = 0;   /* mark as unset */
 	int bytesPerSector = 256;
 	int tpi = 48;		/* 48 tpi */
 	int density = 1;	/* double density */
@@ -185,17 +185,12 @@ int os9format(int argc, char **argv)
 
 					case 't':
 						sectorsPerTrack = atoi(p + 2);
-						// Change sectors on track 0 only if it's not already been changed.
-						if (DEF_SECTORS_TRACK == sectorsTrack0)
-						{
-							sectorsTrack0 = sectorsPerTrack;
-						}
 						break;
-					
+
 					case 'z':
 						sectorsTrack0 = atoi(p + 2);
 						break;
-						
+
 					default:
 						fprintf(stderr,
 							"%s: unknown option '%c'\n",
@@ -245,6 +240,12 @@ int os9format(int argc, char **argv)
 		}
 	}
 
+	/* match sectors on track 0 only if it's unset. */
+	if (sectorsTrack0 == 0)
+	{
+		sectorsTrack0 = sectorsPerTrack;
+	}
+
 	/* if logicalSectors != 0, then format as a hard drive image */
 	if (logicalSectors != 0)
 	{
@@ -270,12 +271,11 @@ int os9format(int argc, char **argv)
 			if (logicalSectors % i == 0)
 			{
 				sectorsPerTrack = logicalSectors / i;
+				sectorsTrack0 = sectorsPerTrack;
 				heads = i;
 				i = 0;
 			}
 		}
-
-		sectorsTrack0 = sectorsPerTrack;
 	}
 
 	/* walk command line for pathnames */
@@ -291,7 +291,7 @@ int os9format(int argc, char **argv)
 
 			error_code ec =
 				_os9_format(argv[i], os968k, tracks,
-					    sectorsPerTrack,sectorsTrack0,
+					    sectorsPerTrack, sectorsTrack0,
 					    heads, bytesPerSector, &clusterSize,
 					    diskName, sectorAllocationSize,
 					    tpi, density, formatEntire,

unittest/librbftest.c

@@ -35,9 +35,9 @@ void test_os9_format()
 	//   (gdb) b _os9_format
 	//   (gdb) run format test.dsk -l65000
 	// Breakpoint 1, _os9_format (pathname=0x7fffffffe0ad "test.dsk", os968k=0,
-	// tracks=125, sectorsPerTrack=4, sectorsTrack0=4, heads=130, sectorSize=256, 
+	// tracks=125, sectorsPerTrack=4, sectorsTrack0=4, heads=130, sectorSize=256,
 	// clusterSize=0x7fffffffdae8, diskName=0x5555555761df "CoCo Disk",
-	// sectorAllocationSize=8, tpi=0, density=0, formatEntire=0, isDragon=0, isHDD=1, 
+	// sectorAllocationSize=8, tpi=0, density=0, formatEntire=0, isDragon=0, isHDD=1,
 	// totalSectors=0x7fffffffdaec, totalBytes=0x7fffffffdaf0) at ../../../librbf/librbfformat.c:25
 	//   (gdb) cont
 	// Continuing.
@@ -54,6 +54,9 @@ void test_os9_format()
 	//   Size in bytes: 16640000
 	//    Cluster size: 1
 
+    // See test_os9_command_format for a test of the command line
+    // that generates this scenario.
+
 	ec = _os9_format("test.dsk", /*68k=*/0,
 			/*tracks=*/125, /*sPT=*/4, /*sT0=*/4, /*heads=*/130, /*sSize=*/256, &clusterSize,
 			 "HD-Test",
@@ -63,30 +66,6 @@ void test_os9_format()
 	ASSERT_EQUALS(65000, totalSectors);
 	ASSERT_EQUALS(65000 * 256, totalBytes);
 
-#ifdef unix
-	// In order to demonstrate the bug described in
-	//     https://github.com/nitros9project/toolshed/issues/28
-	// we have to execute the `os9 format` command.
-	// The bug is not in _os9_format, but in preparing its parameters.
-	ec = system("rm -f test.dsk; ../os9/os9 format test.dsk -l65000");
-	ASSERT_EQUALS(0, ec);
-	// When `os9 format` was broken, it would succeed, but later
-	// operations would fail, like `os9 makdir`:
-	ec = _os9_makdir("test.dsk,/CMDS");
-	ASSERT_EQUALS(0, ec);
-	/*
-		// ... But if `os9format` were in the tinytest linkage, this could work,
-		// instead of using system():
-		char* argv[] = {
-			"os9", "format", "test.dsk", "-l65000", NULL
-		};
-		ec = os9format(4, argv);
-		ASSERT_EQUALS(0, ec);
-		ec = _os9_makdir("test.dsk,/CMDS");
-		ASSERT_EQUALS(0, ec);
-	*/
-#endif
-
 	// TODO: test format with oddball parameters to make sure it can survive
 }
 

unittest/os9commandtest.c

@@ -0,0 +1,43 @@
+/*
+ * ALWAYS BE TESTING!!!
+ *
+ * ALWAYS BE WRITING MORE TESTS!!!
+ *
+ * THIS WORK IS **NEVER** DONE!!!
+ */
+
+#include "tinytest.h"
+#include <toolshed.h>
+
+void test_os9_command_format()
+{
+	error_code ec;
+	native_path_id nativepath;
+	u_int size;
+
+	ec = system("../os9/os9 format -e test.dsk -l65000 > /dev/null 2>&1");
+	ASSERT_EQUALS(0, ec);
+
+	ec = _native_open(&nativepath, "test.dsk", FAM_READ);
+	ASSERT_EQUALS(0, ec);
+
+	ec = _native_gs_size(nativepath, &size);	
+	ASSERT_EQUALS(0, ec);
+	ASSERT_EQUALS(65000*256, size);
+
+	ec = _native_close(nativepath);
+	ASSERT_EQUALS(0, ec);
+
+	ec = _os9_makdir("test.dsk,/CMDS");
+	ASSERT_EQUALS(0, ec);
+}
+
+int main()
+{
+	remove("test.dsk");
+	RUN(test_os9_command_format);
+
+	remove("test.dsk");
+
+	return TEST_REPORT();
+}