Fix dos33.c getopt for BSAVE arguments

Fix dos33.c getopt for BSAVE arguments on non-linux systems, https://github.com/deater/dos33fsprogs/issues/18

tested on OSX and Debian arm64
bugfix display of length when -d debug is used
update doc usage arguments, <> pairs should be []

This succeeds testing on OSX but fails on Linux, work in progress

Tested on Linux and MacOSX arm64, with and without optional BSAVE -l and -a, arguments, with and without local and apple filename and other arguments like CATALOG.

Also verifies this fixes Makefile targets that use dos33, for example project in demos/fire, on OSX currently fails:

$ make
ca65 -o fire.o fire.s -l fire.lst
ld65 -o FIRE fire.o -C ../../linker_scripts/apple2_1000.inc
ca65 -o fire_tiny.o fire_tiny.s -l fire_tiny.lst
ld65 -o FIRE_TINY fire_tiny.o -C ../../linker_scripts/apple2_70.inc
ca65 -o fire_firmware.o fire_firmware.s -l fire_firmware.lst
ld65 -o FIRE_FIRMWARE fire_firmware.o -C ../../linker_scripts/apple2_70.inc
ca65 -o fire_extreme.o fire_extreme.s -l fire_extreme.lst
ld65 -o FIRE_EXTREME fire_extreme.o -C ../../linker_scripts/apple2_70.inc
ca65 -o fire_qkumba.o fire_qkumba.s -l fire_qkumba.lst
ld65 -o FIRE_QKUMBA fire_qkumba.o -C ../../linker_scripts/apple2_70.inc
../../utils/asoft_basic-utils/tokenize_asoft < hello.bas > HELLO
ca65 -o cool_effect.o cool_effect.s -l cool_effect.lst
ld65 -o COOL_EFFECT cool_effect.o -C ../../linker_scripts/apple2_70.inc
ca65 -o raster.o raster.s -l raster.lst
ld65 -o RASTER raster.o -C ../../linker_scripts/apple2_70.inc
ld65 -o FIRE_ELSEWHERE fire_extreme.o -C ../../linker_scripts/apple2_c0.inc
../../utils/dos33fs-utils/dos33 -y fire.dsk SAVE A HELLO
Warning!  HELLO exists!
Deleting previous version...
../../utils/dos33fs-utils/dos33 -y fire.dsk BSAVE -a 0x1000 FIRE
Error!  First char of filename must be ASCII 64 or above!
make: *** [fire.dsk] Error 255

With this change, it now succeeds:

$ make
ca65 -o fire.o fire.s -l fire.lst
ld65 -o FIRE fire.o -C ../../linker_scripts/apple2_1000.inc
ca65 -o fire_tiny.o fire_tiny.s -l fire_tiny.lst
ld65 -o FIRE_TINY fire_tiny.o -C ../../linker_scripts/apple2_70.inc
ca65 -o fire_firmware.o fire_firmware.s -l fire_firmware.lst
ld65 -o FIRE_FIRMWARE fire_firmware.o -C ../../linker_scripts/apple2_70.inc
ca65 -o fire_extreme.o fire_extreme.s -l fire_extreme.lst
ld65 -o FIRE_EXTREME fire_extreme.o -C ../../linker_scripts/apple2_70.inc
ca65 -o fire_qkumba.o fire_qkumba.s -l fire_qkumba.lst
ld65 -o FIRE_QKUMBA fire_qkumba.o -C ../../linker_scripts/apple2_70.inc
../../utils/asoft_basic-utils/tokenize_asoft < hello.bas > HELLO
ca65 -o cool_effect.o cool_effect.s -l cool_effect.lst
ld65 -o COOL_EFFECT cool_effect.o -C ../../linker_scripts/apple2_70.inc
ca65 -o raster.o raster.s -l raster.lst
ld65 -o RASTER raster.o -C ../../linker_scripts/apple2_70.inc
ld65 -o FIRE_ELSEWHERE fire_extreme.o -C ../../linker_scripts/apple2_c0.inc
../../utils/dos33fs-utils/dos33 -y fire.dsk SAVE A HELLO
Warning!  HELLO exists!
Deleting previous version...
../../utils/dos33fs-utils/dos33 -y fire.dsk BSAVE -a 0x1000 FIRE
Warning!  FIRE exists!
Deleting previous version...
../../utils/dos33fs-utils/dos33 -y fire.dsk BSAVE -a 0x70 FIRE_TINY
Warning!  FIRE_TINY exists!
Deleting previous version...
../../utils/dos33fs-utils/dos33 -y fire.dsk BSAVE -a 0x70 FIRE_FIRMWARE
Warning!  FIRE_FIRMWARE exists!
Deleting previous version...
../../utils/dos33fs-utils/dos33 -y fire.dsk BSAVE -a 0x70 FIRE_EXTREME
Warning!  FIRE_EXTREME exists!
Deleting previous version...
../../utils/dos33fs-utils/dos33 -y fire.dsk BSAVE -a 0x70 FIRE_QKUMBA
Warning!  FIRE_QKUMBA exists!
Deleting previous version...
../../utils/dos33fs-utils/dos33 -y fire.dsk BSAVE -a 0x70 RASTER
Warning!  RASTER exists!
Deleting previous version...
../../utils/dos33fs-utils/dos33 -y fire.dsk BSAVE -a 0x70 COOL_EFFECT
Warning!  COOL_EFFECT exists!
Deleting previous version...
../../utils/dos33fs-utils/dos33 -y fire.dsk BSAVE -a 0xC0 FIRE_ELSEWHERE
Warning!  FIRE_ELSEWHERE exists!
Deleting previous version...

Hi @jquast, and thanks for giving attention to my issue! However, I feel like your current solution is perhaps a bit fragile - it's going to stop working in the future if BSAVE ever gets a non-argument option, because then all of a sudden getopt's "option-cuddling" will not be supported.

As mentioned in #18, there's no reason to avoid using getopt to process those args, and continuing to use getopt would keep the code much simpler. You can simply use getopt to process the general args until it returns -1, then if !strcmp(*argv, "BSAVE"), you can ++argv and go right back to using getopt to process the options to BSAVE. That strikes me as a more robust (future-proof) solution, and should also involve a lot less logic duplication (eliminating the need for lines 1245-1284 in the current version of your branch).

Cheers!

@micahcowan Sorry that it took so long to reply, it is because I could not implement the code like you request.

BSD and Linux differ very much in regards to getopt(3), and it is suggested to always use getopt_long(3) where compatibility matters, but that would require changing the command line api of this program.

While I was able to use getopt a second time, please review, it does require re-scanning for position of BSAVE in argv after it is mutated and resetting optind, and, re-parsing for -a and -l arguments in the second pass where it is required on BSD but not with linux.

All of this is described with a very long comment inline, but I will re-describe the most important issue here.

Linux mutates argv after processing, while bsd does not, and this causes any second-pass processing of argv to be unreliable to perform on both systems.

In getopt call,

while ((c = getopt(argc, argv, "a:l:t:s:dhvxy")) != -1)

With example command, ../dos33 -d test.dsk BSAVE -a 0x100 -l 1 SINCOS abc
On linux, argv is mutated,
```
argv[0]=../dos33
argv[1]=-d
```
argv[2]=test.dsk
argv[3]=BSAVE
argv[4]=-a
argv[5]=0x100
argv[6]=-l
argv[7]=1
argv[2]=-a
argv[3]=0x100
argv[4]=-l
argv[5]=1
argv[6]=test.dsk
argv[7]=BSAVE argv[8]=SINCOS argv[9]=abc

optind becomes 6, pointing at test.dsk, then incremented to become command "BSAVE" and remaining arguments, local file "SINCOS" and apple file "abc".

Also exclusive to linux, when l:a: is removed from the getopt string, getopt will cause stderr message, ../dos33: invalid option -- 'a' to display, though it does continue parsing. While this message does not display on BSD, as it has stopped at BSAVE command before reaching them. Linux also mutates argv into something totally unusable in this case: ["../dos33", "-d", "-a", "-l", "test.dsk", "BSAVE", "0x100", "1", "SINCOS", "abc"]

While on BSD, things are much simpler, argv is not mutated, and l:a: can be included or excluded in first getopt call in the given code without error, and optind is 2, pointing at test.dsk with remaining arguments unmodified for re-processing by a second pass of getopt, "BSAVE", "-a", "-x100", "-l", "1", "SINCOS", and "abc".

@jquast Before we go any further, I should maybe point out that in my experience @deater hasn't had much of a tendency to respond to or incorporate pull requests, so probably won't see this anyway (I'm sorry I didn't point this out earlier, before you'd done the work).

I'm not really clear on why GNU getopt's permuation of argv should present a problem for my proposed solution that re-invokes getopt, but somehow not for yours which does the same... not least because no actual reinvocation is necessary on the platform that actually does this permutation.

But I guess it's time to stop explaining with words, and start showing code. #23 demonstrates the concept I meant. It's probably not perfect, but I believe it has the following advantages over this implementation (#22):

It avoids redundant/repeated logic for separate getopt-invocation spots (that can easily get “out of sync” if changes are made).
Scalable: avoids things getting rather out of hand if other commands besides BSAVE gain options in the future (say, a -v to CATALOG for extra info, or something).
No rescanning argv from the top to search for "BSAVE" or other commands (that wasn't actually necessary in yours, either: the command will always be at argv[optind+1] when getopt exits with -1, regardless of whether permutations occurred or not. Otherwise dos33 wouldn't have been able to find the command in the first place, to handle BSAVE).
Requires less added code overall.

If you see the merit in this approach, please feel free to take it over and adapt it in your (this) pull request. Mine lacks your regression test, and could (IMO) be improved by moving the whole getopt stuff out into a function that gets called twice, rather than using an extra loop.

deater / dos33fsprogs

Fix dos33.c getopt for BSAVE arguments #22