| Re: [hatari-devel] Hatari GEMDOS drive volume name |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/hatari-devel Archives
]
Hi Uwe,
On 8.9.2024 11.45, Uwe Seimet wrote:
if(!Fsfirst("C:\*.*", FA_VOLUME)) {
DTA *dta = Fgetdta();
if(dta->d_attrib == 0x08) {
// Unexpected: d_attrib is $00 here, shouldn't it be 0x08?
// d_fname is "EMULATED.002"
}
}
Thanks for reporting!
Fix for that is one-liner, but the attached patch should fix also few
other GEMDOS HD attribute handling corner-cases I noticed while
investigating this.
=> It's good if you could test that it does not regress anything else
before I commit it.
(Hatari GEMDOS HD unit test are very basic, as is my setup & manual
testing, so you're likely to catch things I do not.)
Thomas / Nicolas, does the patch look OK?
- Eero
From 75a33df5b164d965540058e35eb6fcc7f3496357 Mon Sep 17 00:00:00 2001
From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
Date: Sun, 8 Sep 2024 16:17:01 +0300
Subject: [PATCH] GEMDOS HD: fixes for attribute handling corner-cases
Instead of using in Fsnext() a (global) attribute value set by
previous Fsfirst() call, store it to internal DTA entry. This fixes
case where program first Fgetdata()s multiple DTAs with _different_
attributes from Fsfirst() calls, and then does Fsnext() calls while
switching between those earlier acquired DTAs with Fsetdta().
Support volume labels also in case when archive and/or read-only
attributes are specified, with the new IS_VOLUME_LABEL() macro.
Use correct attribute value also for volume labels returned from Fsfirst().
Improve related code comments & warning messages and update docs.
---
doc/manual.html | 2 ++
doc/release-notes.txt | 3 +++
src/gemdos.c | 47 +++++++++++++++++++++--------------
src/includes/gemdos_defines.h | 3 ++-
4 files changed, 35 insertions(+), 20 deletions(-)
diff --git a/doc/manual.html b/doc/manual.html
index 2e26c4bd..4c137ab9 100644
--- a/doc/manual.html
+++ b/doc/manual.html
@@ -2707,6 +2707,8 @@ have no files.</li>
handles used with GEMDOS <em>file</em> functions, such as Fwrite() to standard
output that is redirected to a file. File redirection is NOT supported
e.g. for GEMDOS Ccon* console functions.</li>
+<li>Only FsFirst() / FsNext() calls support drive volume labels, other
+GEMDOS calls return errors when volume label attribute is specified
<li>Host and emulation have their own clocks, which will drift apart
after emulation is started, especially when emulation is
fast-forwarded or paused. Because of this, anything inside emulation
diff --git a/doc/release-notes.txt b/doc/release-notes.txt
index e072fafb..17fc1758 100644
--- a/doc/release-notes.txt
+++ b/doc/release-notes.txt
@@ -46,6 +46,9 @@ Emulator improvements:
- Fix: segfault with negative joystick indexes
- GEMDOS HD:
- Fix: Fattrib() call on a directory
+ - Fix: Return attrib for volume labels in FsFirst()
+ - Fix: Fsnext() calls with DTAs having different attribs,
+ where DTA is not from preceeding Fsfirst() but earlier one
- Fix: Handle additional '*' chars at end of file mask
- SCC:
- Fix: invalid channel B file close on uninit
diff --git a/src/gemdos.c b/src/gemdos.c
index 8bbf1dc1..7a840fd7 100644
--- a/src/gemdos.c
+++ b/src/gemdos.c
@@ -141,6 +141,7 @@ typedef struct
char szActualName[MAX_GEMDOS_PATH]; /* used by F_DATIME (0x57) */
} FILE_HANDLE;
+/* stored FsFirst() information */
typedef struct
{
bool bUsed;
@@ -148,16 +149,20 @@ typedef struct
int nentries; /* number of entries in fs directory */
int centry; /* current entry # */
struct dirent **found; /* legal files */
- char path[MAX_GEMDOS_PATH]; /* sfirst path */
+ char path[MAX_GEMDOS_PATH];
+ char dta_attrib;
} INTERNAL_DTA;
+/* DTA population ignores archive & read-only attributes */
+#define IGNORED_FILE_ATTRIBS (GEMDOS_FILE_ATTRIB_WRITECLOSE|GEMDOS_FILE_ATTRIB_READONLY)
+#define IS_VOLUME_LABEL(x) (((x) & ~(IGNORED_FILE_ATTRIBS)) == GEMDOS_FILE_ATTRIB_VOLUME_LABEL)
+
static FILE_HANDLE FileHandles[MAX_FILE_HANDLES];
static INTERNAL_DTA *InternalDTAs;
static int DTACount; /* Current DTA cache size */
static uint16_t DTAIndex; /* Circular index into above */
static uint16_t CurrentDrive; /* Current drive (0=A,1=B,2=C etc...) */
static uint32_t act_pd; /* Used to get a pointer to the current basepage */
-static uint16_t nAttrSFirst; /* File attribute for SFirst/Snext */
static uint32_t CallingPC; /* Program counter from caller */
static uint32_t nSavedPexecParams;
@@ -287,7 +292,7 @@ static uint8_t GemDOS_ConvertAttribute(mode_t mode, const char *path)
/* TODO, Other attributes:
* - GEMDOS_FILE_ATTRIB_HIDDEN (file not visible on desktop/fsel)
- * - GEMDOS_FILE_ATTRIB_ARCHIVE (file written after being backed up)
+ * - GEMDOS_FILE_ATTRIB_WRITECLOSE (arcive bit, file written after being backed up)
* ?
*/
return Attrib;
@@ -299,7 +304,7 @@ static uint8_t GemDOS_ConvertAttribute(mode_t mode, const char *path)
* Populate the DTA buffer with file info.
* @return DTA_OK if entry is ok, DTA_SKIP if it should be skipped, DTA_ERR on errors
*/
-static dta_ret_t PopulateDTA(char *path, struct dirent *file, DTA *pDTA, uint32_t DTA_Gemdos)
+static dta_ret_t PopulateDTA(INTERNAL_DTA *iDTA, struct dirent *file, DTA *pDTA, uint32_t DTA_Gemdos)
{
/* TODO: host file path can be longer than MAX_GEMDOS_PATH */
char tempstr[MAX_GEMDOS_PATH];
@@ -307,8 +312,8 @@ static dta_ret_t PopulateDTA(char *path, struct dirent *file, DTA *pDTA, uint32_
DATETIME DateTime;
int nFileAttr, nAttrMask;
- if (snprintf(tempstr, sizeof(tempstr), "%s%c%s",
- path, PATHSEP, file->d_name) >= (int)sizeof(tempstr))
+ if (snprintf(tempstr, sizeof(tempstr), "%s%c%s", iDTA->path,
+ PATHSEP, file->d_name) >= (int)sizeof(tempstr))
{
Log_Printf(LOG_ERROR, "PopulateDTA: path is too long.\n");
return DTA_ERR;
@@ -327,7 +332,7 @@ static dta_ret_t PopulateDTA(char *path, struct dirent *file, DTA *pDTA, uint32_
/* Check file attributes (check is done according to the Profibuch) */
nFileAttr = GemDOS_ConvertAttribute(filestat.st_mode, tempstr);
- nAttrMask = nAttrSFirst|GEMDOS_FILE_ATTRIB_WRITECLOSE|GEMDOS_FILE_ATTRIB_READONLY;
+ nAttrMask = iDTA->dta_attrib | IGNORED_FILE_ATTRIBS;
if (nFileAttr != 0 && !(nAttrMask & nFileAttr))
return DTA_SKIP;
@@ -1978,10 +1983,9 @@ static bool GemDOS_Create(uint32_t Params)
return redirect_to_TOS();
}
- if (Mode == GEMDOS_FILE_ATTRIB_VOLUME_LABEL)
+ if (IS_VOLUME_LABEL(Mode))
{
- Log_Printf(LOG_WARN, "Warning: Hatari doesn't support GEMDOS volume"
- " label setting\n(for '%s')\n", pszFileName);
+ Log_Printf(LOG_WARN, "volume label creation not supported on GEMDOS HD\n");
Regs[REG_D0] = GEMDOS_EFILNF; /* File not found */
return true;
}
@@ -2606,9 +2610,9 @@ static bool GemDOS_Fattrib(uint32_t Params)
GemDOS_CreateHardDriveFileName(nDrive, psFileName,
sActualFileName, sizeof(sActualFileName));
- if (nAttrib == GEMDOS_FILE_ATTRIB_VOLUME_LABEL)
+ if (IS_VOLUME_LABEL(nAttrib))
{
- Log_Printf(LOG_WARN, "Hatari doesn't support GEMDOS volume label setting\n(for '%s')\n", sActualFileName);
+ Log_Printf(LOG_WARN, "volume label Fattrib() not supported on GEMDOS HD\n");
Regs[REG_D0] = GEMDOS_EFILNF; /* File not found */
return true;
}
@@ -2914,7 +2918,7 @@ static bool GemDOS_SNext(void)
return false;
}
- if (nAttrSFirst == GEMDOS_FILE_ATTRIB_VOLUME_LABEL)
+ if (IS_VOLUME_LABEL(pDTA->dta_attrib))
{
/* Volume label was given already in Sfirst() */
Regs[REG_D0] = GEMDOS_ENMFIL;
@@ -2946,7 +2950,7 @@ static bool GemDOS_SNext(void)
return true;
}
- ret = PopulateDTA(InternalDTAs[Index].path,
+ ret = PopulateDTA(&InternalDTAs[Index],
temp[InternalDTAs[Index].centry++],
pDTA, DTA_Gemdos);
} while (ret == DTA_SKIP);
@@ -2981,17 +2985,18 @@ static bool GemDOS_SFirst(uint32_t Params)
DTA *pDTA;
uint32_t DTA_Gemdos;
uint16_t useidx;
+ uint16_t attrib;
- nAttrSFirst = STMemory_ReadWord(Params+SIZE_LONG);
+ attrib = STMemory_ReadWord(Params+SIZE_LONG);
pszFileName = STMemory_GetStringPointer(STMemory_ReadLong(Params));
if (!pszFileName)
{
LOG_TRACE(TRACE_OS_GEMDOS, "GEMDOS 0x4E bad Fsfirst(0x%X, 0x%x) at PC 0x%X\n",
- STMemory_ReadLong(Params), nAttrSFirst, CallingPC);
+ STMemory_ReadLong(Params), attrib, CallingPC);
return false;
}
- LOG_TRACE(TRACE_OS_GEMDOS, "GEMDOS 0x4E Fsfirst(\"%s\", 0x%x) at PC 0x%X\n", pszFileName, nAttrSFirst,
+ LOG_TRACE(TRACE_OS_GEMDOS, "GEMDOS 0x4E Fsfirst(\"%s\", 0x%x) at PC 0x%X\n", pszFileName, attrib,
CallingPC);
Drive = GemDOS_FileName2HardDriveID(pszFileName);
@@ -3045,13 +3050,17 @@ static bool GemDOS_SFirst(uint32_t Params)
}
InternalDTAs[useidx].bUsed = true;
InternalDTAs[useidx].addr = DTA_Gemdos;
+ InternalDTAs[useidx].dta_attrib = attrib;
- /* Were we looking for the volume label? */
- if (nAttrSFirst == GEMDOS_FILE_ATTRIB_VOLUME_LABEL)
+ /* volume labels are supported only when no other
+ * other file types are specified in same query
+ */
+ if (IS_VOLUME_LABEL(attrib))
{
/* Volume name */
strcpy(pDTA->dta_name,"EMULATED.001");
pDTA->dta_name[11] = '0' + Drive;
+ pDTA->dta_attrib = GEMDOS_FILE_ATTRIB_VOLUME_LABEL;
Regs[REG_D0] = GEMDOS_EOK; /* Got volume */
return true;
}
diff --git a/src/includes/gemdos_defines.h b/src/includes/gemdos_defines.h
index 53352925..80fcb126 100644
--- a/src/includes/gemdos_defines.h
+++ b/src/includes/gemdos_defines.h
@@ -53,7 +53,8 @@
#define GEMDOS_FILE_ATTRIB_SYSTEM_FILE 0x04
#define GEMDOS_FILE_ATTRIB_VOLUME_LABEL 0x08
#define GEMDOS_FILE_ATTRIB_SUBDIRECTORY 0x10
-/* file was written and closed correctly (used automatically on gemdos >=0.15) */
+/* archive bit, file was written and closed correctly
+ * (used automatically on gemdos >=0.15) */
#define GEMDOS_FILE_ATTRIB_WRITECLOSE 0x20
#endif /* HATARI_GEMDOS_DEFINES_H */
--
2.39.2