Re: [hatari-devel] Regression in Hatari 2.6.1 - Disabling GEMDOS drive fails

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi Christian,

(Did not receive Nicolas' reply, but I saw it in archive.)

On 31.8.2025 11.36, Christian Zietz wrote:
> sadly, I have to report a regression in Hatari 2.6.1. The manual states 
> about the -d / --harddrive option: "If <dir> is an empty string, then 
> harddrive's emulation is disabled". This worked well until and including 
> Hatari 2.6.0. In Hatari 2.6.1, however:
>
> hatari-2.6.1_windows64\hatari.exe -d ""
>
> Hatari v2.6.1 - the Atari ST, STE, TT and Falcon emulator.
>
> Hatari is free software licensed under the GNU General Public License.
>
> Usage:
>   hatari [options] [disk image name]
>
> Try option "-h" or "--help" to display more information.
>
> Error while parsing argument "" for option "--harddrive":
>    Given directory doesn't exist or permissions prevent access to it!

Aargh, I forgot about this case when my fix for the screenshot change 
issues expanded to:
https://framagit.org/hatari/hatari/-/commit/77b02c484c599856c57edf1b75b66b1c9d6ddda0

My commit changed options to use same code for dir options parsing as 
was used for files. This means that <dir> options are now disabled same 
way as <file> options, by giving "none" string.

I kind of like that consistency although it's a CLI API change.

What do you think, would updating docs be enough (see attached patch)?


	- Eero

From 2d966095fe1cc9b64c904acbfb45ffa779d9e9e4 Mon Sep 17 00:00:00 2001
From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
Date: Wed, 3 Sep 2025 00:38:29 +0300
Subject: [PATCH] Document GEMDOS HD dir option change

Fixes: 77b02c484c599856c

Reported-by: Christian Zietz
---
 doc/hatari.1          |  4 ++--
 doc/manual.html       |  4 ++--
 doc/release-notes.txt |  7 +++++++
 src/options.c         | 20 +++++++++++---------
 4 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/doc/hatari.1 b/doc/hatari.1
index bd1b5af2..bb6f5761 100644
--- a/doc/hatari.1
+++ b/doc/hatari.1
@@ -394,8 +394,8 @@ If directory contains only single letter (C-Z) subdirectories, each of these
 subdirectories will be treated as a separate partition, otherwise the
 given directory itself will be assigned to drive "C:". In the multiple
 partition case, the letters used as the subdirectory names will
-determine to which drives/partitions they are assigned. If <dir> is
-an empty string, then harddrive's emulation is disabled
+determine to which drives/partitions they are assigned to. If "none"
+is given as <dir>, then GEMDOS HD emulation is disabled
 .TP
 .B \-\-protect\-hd <x>
 Write protect harddrive <dir> contents (on/off/auto). With "auto" option
diff --git a/doc/manual.html b/doc/manual.html
index 2b44df2c..8c2855d8 100644
--- a/doc/manual.html
+++ b/doc/manual.html
@@ -926,8 +926,8 @@ with &lt;dir&gt; contents. If directory contains only single letter
 as a separate partition, otherwise the given directory itself will
 be assigned to drive "C:". In the multiple partition case, the
 letters used as the subdirectory names will determine to which
-drives/partitions they&rsquo;re assigned. If &lt;dir&gt; is
-an empty string, then harddrive's emulation is disabled</p>
+drives/partitions they are assigned to. If "none" is given as
+&lt;dir&gt;, then GEMDOS HD emulation is disabled</p>
 <p class="parameter">--protect-hd
 &lt;x&gt;</p>
 <p class="paramdesc">Write protect hard drive &lt;dir&gt;
diff --git a/doc/release-notes.txt b/doc/release-notes.txt
index c898fc51..6b890c30 100644
--- a/doc/release-notes.txt
+++ b/doc/release-notes.txt
@@ -37,10 +37,17 @@ Emulator improvements:
     by adding '\' to end of line to continue it
   - Increase max lenght of accepted debugger file commands to 4k
 
+Documentation:
+- Document v2.6.1 API change
+
 
  Version 2.6.1 (2025-08-15)
  --------------------------
 
+API changes:
+- GEMDOS HD emulation is disabled from CLI by providing "none" as
+  <dir> instead of an empty string, same as with device <file> options
+
 Emulation improvements:
 - MegaSTE:
   - When using cache, only compare 24 lowest bits of address
diff --git a/src/options.c b/src/options.c
index 89f3a863..28abc2c3 100644
--- a/src/options.c
+++ b/src/options.c
@@ -59,10 +59,15 @@ bool BenchmarkMode;	   /* Start in benchmark mode (try to run at maximum emulati
 
 static bool bBiosIntercept;
 
+/* Opt_StrCpy() option types.
+ *
+ * If both a dir/file string and a bool enabling given device
+ * are provided, "none" as dir/file string disables given device
+ */
 typedef enum {
-	CHECK_NONE,
-	CHECK_FILE,
-	CHECK_DIR
+	CHECK_NONE,	/* create file if it does not exist */
+	CHECK_FILE,	/* given file needs to exist */
+	CHECK_DIR	/* given dir needs to exist */
 } fs_check_t;
 
 
@@ -679,7 +684,7 @@ static void Opt_ShowHelp(void)
 	       "\tEnable by using 'y', 'yes', 'on', 'true' or '1'\n"
 	       "<file>\tDevices accept also special 'stdout' and 'stderr' file names\n"
 	       "\t(if you use stdout for midi or printer, set log to stderr).\n"
-	       "\tSetting the file to 'none', disables given device or disk\n");
+	       "<dir>/<file>\t'none' disables given device / disk.\n");
 }
 
 
@@ -941,8 +946,8 @@ static int Opt_WhichOption(int argc, const char * const argv[], int idx)
  * When 'check' is set, check whether given 'src' exists before copying
  * 'src' to 'dst'. Otherwise just copy option 'src' string to 'dst'.
  * If a pointer to (bool) 'option' is given, set that option to true.
- * - However, if src is "none", leave dst unmodified & set option to false.
- *   ("none" is used to disable options related to file arguments)
+ * - However, if src is "none", leave 'dst' unmodified & set 'option'
+ *   to false. (i.e. disable bool that enables given device)
  * Return false if there were errors, otherwise true
  */
 static bool Opt_StrCpy(int optid, fs_check_t check, char *dst, const char *src, size_t dstlen, bool *option)
@@ -1164,7 +1169,6 @@ bool Opt_ParseParameters(int argc, const char * const argv[])
 
 		case OPT_CONFIGFILE:
 			i += 1;
-			/* true -> file needs to exist */
 			ok = Opt_StrCpy(OPT_CONFIGFILE, CHECK_FILE, sConfigFileName,
 					argv[i], sizeof(sConfigFileName), NULL);
 			if (ok)
@@ -1384,7 +1388,6 @@ bool Opt_ParseParameters(int argc, const char * const argv[])
 
 		case OPT_AVIRECORD_FILE:
 			i += 1;
-			/* false -> file is created if it doesn't exist */
 			ok = Opt_StrCpy(OPT_AVIRECORD_FILE, CHECK_NONE, ConfigureParams.Video.AviRecordFile,
 					argv[i], sizeof(ConfigureParams.Video.AviRecordFile), NULL);
 			break;
@@ -1505,7 +1508,6 @@ bool Opt_ParseParameters(int argc, const char * const argv[])
 
 		case OPT_PRINTER:
 			i += 1;
-			/* "none" can be used to disable printer */
 			ok = Opt_StrCpy(OPT_PRINTER, CHECK_NONE, ConfigureParams.Printer.szPrintToFileName,
 					argv[i], sizeof(ConfigureParams.Printer.szPrintToFileName),
 					&ConfigureParams.Printer.bEnablePrinting);
-- 
2.39.5