FileSwitch 2.90 introduced a register corruption bug in *Copy which would (typically) result in an abort if the copy isn't going ahead because the destination exists. This was down to the (undocumented) input of R7 being corrupted by the call to EnsureCanonicalObject, and not being restored before it gets used later on by the call to Util_PrintName.
Fix the code to restore R7 before calling Util_PrintName, and update the comments surrounding a couple of functions to make sure at least some of the undocumented inputs are documented.
Fixes issue reported on forums: https://www.riscosopen.org/forum/forums/4/topics/17802
ROOL (8cdba019) at 25 Mar 11:19
Fix register corruption in CopyFile_EnsureDestFileWriteable
It looks like the problems mentioned in that thread are down to JetDirectFS & UniServerFS claiming to support fsfile_ReadInfoNoLen (by having bit 26 of their FS information word set), when in reality they don't support it.
OK, sounds like it's the "caller's fault" situation, so nothing to do here.
R-Comp got in touch with me about that last night. It looks like the problems mentioned in that thread are down to JetDirectFS & UniServerFS claiming to support fsfile_ReadInfoNoLen (by having bit 26 of their FS information word set), when in reality they don't support it. Since FileSwitch hasn't attempted to use fsfile_ReadInfoNoLen for decades, it's pretty easy for a bug like that to have gone unnoticed until now.
There was a possibly related report which may have coincided with FileSwitch 2.91 and sounds like path handling changed with JetDirectFS. Any idea if that's the caller's fault (by assuming something about the API which was an accident) or FileSwitch no longer providing the length in some situation?
FileSwitch 2.90 introduced a register corruption bug in *Copy which would (typically) result in an abort if the copy isn't going ahead because the destination exists. This was down to the (undocumented) input of R7 being corrupted by the call to EnsureCanonicalObject, and not being restored before it gets used later on by the call to Util_PrintName.
Fix the code to restore R7 before calling Util_PrintName, and update the comments surrounding a couple of functions to make sure at least some of the undocumented inputs are documented.
Fixes issue reported on forums: https://www.riscosopen.org/forum/forums/4/topics/17802
One of the fixes in PipeFS 0.25 was to fix the fsfile_ReadInfo
operation
to block until the pipe is no longer open for write (instead of failing
with a "file not found" error, as it had done previously). This fix had
some knock-ons for FileSwitch, since the contemporary version (2.89)
only made use of fsfile_ReadInfo
. E.g.
trying to open a pipe for read while it's open for write would block due
to TopPath_DoBusinessToPath
using fsfile_ReadInfo
when FileSwitch is checking whether the file exists. This specific issue
was fixed in FileSwitch 2.90 by changing some areas of FileSwitch to use
fsfile_ReadInfoNoLen
instead.
Further testing has revealed a few more areas which are making unnecessary use of fsfile_ReadInfo and have the potential to break programs. Specifically, with FileSwitch 2.90
& PipeFS 0.25, programs which use GCC's UnixLib will get stuck if they
try to open a PipeFS file for write while running in a taskwindow. E.g.
the call to GNU.gawk in Env.!Common. This happens because after opening
the file, UnixLib attempts to set its attributes using OS_File 4, which
in turn causes FileSwitch to attempt to examine the file using
fsfile_ReadInfo
.
This MR fixes that above issue and a few other potential issues by changing several more areas of FileSwitch to use fsfile_ReadInfoNoLen
where possible:
CanonicalisePathEntry
& RenameEntry
) and ChangeDirectory
helper routineTopPath_DoBusinessForDirectoryRead
ROOL (81d3304d) at 13 Mar 12:46
Change TopPath_DoBusinessForDirectoryRead to use fsfile_ReadInfoNoLen
... and 6 more commits
Merge would be appreciated. Current Rom builds without this fail reliably with task windows 'going nowhere'
One of the fixes in PipeFS 0.25 was to fix the fsfile_ReadInfo
operation
to block until the pipe is no longer open for write (instead of failing
with a "file not found" error, as it had done previously). This fix had
some knock-ons for FileSwitch, since the contemporary version (2.89)
only made use of fsfile_ReadInfo
. E.g.
trying to open a pipe for read while it's open for write would block due
to TopPath_DoBusinessToPath
using fsfile_ReadInfo
when FileSwitch is checking whether the file exists. This specific issue
was fixed in FileSwitch 2.90 by changing some areas of FileSwitch to use
fsfile_ReadInfoNoLen
instead.
Further testing has revealed a few more areas which are making unnecessary use of fsfile_ReadInfo and have the potential to break programs. Specifically, with FileSwitch 2.90
& PipeFS 0.25, programs which use GCC's UnixLib will get stuck if they
try to open a PipeFS file for write while running in a taskwindow. E.g.
the call to GNU.gawk in Env.!Common. This happens because after opening
the file, UnixLib attempts to set its attributes using OS_File 4, which
in turn causes FileSwitch to attempt to examine the file using
fsfile_ReadInfo
.
This MR fixes that above issue and a few other potential issues by changing several more areas of FileSwitch to use fsfile_ReadInfoNoLen
where possible:
CanonicalisePathEntry
& RenameEntry
) and ChangeDirectory
helper routineTopPath_DoBusinessForDirectoryRead
One of the fixes in PipeFS 0.25 was to fix the fsfile_ReadInfo
operation
to block until the pipe is no longer open for write (instead of failing
with a "file not found" error, as it had done previously). This fix had
some knock-ons for FileSwitch, since the contemporary version (2.89)
only made use of fsfile_ReadInfo
. E.g.
trying to open a pipe for read while it's open for write would block due
to TopPath_DoBusinessToPath
using fsfile_ReadInfo
when FileSwitch is checking whether the file exists. This specific issue
was fixed in FileSwitch 2.90 by changing some areas of FileSwitch to use
fsfile_ReadInfoNoLen
instead.
Further testing has revealed a few more areas which are making unnecessary use of fsfile_ReadInfo and have the potential to break programs. Specifically, with FileSwitch 2.90
& PipeFS 0.25, programs which use GCC's UnixLib will get stuck if they
try to open a PipeFS file for write while running in a taskwindow. E.g.
the call to GNU.gawk in Env.!Common. This happens because after opening
the file, UnixLib attempts to set its attributes using OS_File 4, which
in turn causes FileSwitch to attempt to examine the file using
fsfile_ReadInfo
.
This MR fixes that above issue and a few other potential issues by changing several more areas of FileSwitch to use fsfile_ReadInfoNoLen
where possible:
CanonicalisePathEntry
& RenameEntry
) and ChangeDirectory
helper routineTopPath_DoBusinessForDirectoryRead
Update EnsureCanonicalObject so that it can optionally skip reading the file/object length, based on the new argument R7. Then trace back through the callers to have them pass in an appropriate value based on whether they care about the length or not.
TopPath_DoBusinessToPath has also gained a new flag bit to suppress reading the length, but so far I've only updated DoThePath to use it.
Result: If you have a file in PipeFS which is open for writing, and then attempt to open it for reading using OS_Find, it'll now open immediately instead of PipeFS trying to block/sleep until it's no longer open for writing.
Why: Since at least as far back as RISC OS 3.6, PipeFS tries to sleep inside fsfile_ReadInfo if the file is open for writing. But a combination of buggy sleep logic and TaskWindow's lazy approach to UpCall 6 meant that (prior to TaskWindow 0.65) it wouldn't sleep for long and would instead allow the open to proceed while the file is still open for write. The ability to have both a reader & writer active at the same time is basically the whole point of PipeFS, so this change restores that pre-TaskWindow 0.65 behaviour without having to remove the sleep logic from PipeFS (it's a well-intentioned feature, even though it's been broken for decades)
Also now that OS_TaskControl is in the kernel, merge in my change to make FileSwitch use it when starting a program.
Tested by performing a ROM build, checking that it works, and running some PipeFS tests.
This fixes the original issue described in ticket 480: https://www.riscosopen.org/tracker/tickets/480
Update EnsureCanonicalObject so that it can optionally skip reading the file/object length, based on the new argument R7. Then trace back through the callers to have them pass in an appropriate value based on whether they care about the length or not.
TopPath_DoBusinessToPath has also gained a new flag bit to suppress reading the length, but so far I've only updated DoThePath to use it.
Result: If you have a file in PipeFS which is open for writing, and then attempt to open it for reading using OS_Find, it'll now open immediately instead of PipeFS trying to block/sleep until it's no longer open for writing.
Why: Since at least as far back as RISC OS 3.6, PipeFS tries to sleep inside fsfile_ReadInfo if the file is open for writing. But a combination of buggy sleep logic and TaskWindow's lazy approach to UpCall 6 meant that (prior to TaskWindow 0.65) it wouldn't sleep for long and would instead allow the open to proceed while the file is still open for write. The ability to have both a reader & writer active at the same time is basically the whole point of PipeFS, so this change restores that pre-TaskWindow 0.65 behaviour without having to remove the sleep logic from PipeFS (it's a well-intentioned feature, even though it's been broken for decades)
Also now that OS_TaskControl is in the kernel, merge in my change to make FileSwitch use it when starting a program.
Tested by performing a ROM build, checking that it works, and running some PipeFS tests.
This fixes the original issue described in ticket 480: https://www.riscosopen.org/tracker/tickets/480
Ref https://www.riscosopen.org/forum/forums/4/topics/17259
ROOL (5944f979) at 24 Dec 08:23
Fix for NULL pointer dereference with Count/Copy/Wipe ops on duff FS
My 24" widescreen monitor questions a 50 or 70 character limit, but salutes anyone developing on a VT100 terminal in 2022.
Nit: I think you can probably easily slightly reword the subject line to under 70 characters - remember we're already quite generous with this, it's usually recommended to keep them under 50 characters.
Ref https://www.riscosopen.org/forum/forums/4/topics/17259
Detail: