Skip to content

dos: add directory-relative file functions#566

Open
robn wants to merge 8 commits into
aros-development-team:masterfrom
robn:dos-lock-relative
Open

dos: add directory-relative file functions#566
robn wants to merge 8 commits into
aros-development-team:masterfrom
robn:dos-lock-relative

Conversation

@robn

@robn robn commented Apr 28, 2021

Copy link
Copy Markdown
Contributor

This adds extended versions of all DOS functions that take a filename and interpret a relative name against the current working directory to take a directory lock and a filename, and then interpret a relative name against the given lock.

This lets an application avoid race conditions when operating on files outside the current working directory. The application has to assemble path strings to do this, and there's a possibility that one of the intermediate path components could be a link, and another process could retarget it.

This is analogous to the POSIX *at collection of functions (eg openat, linkat, etc). And indeed, that's part of why I want these: so I can cleanly implement the POSIX wrappers without out having to resort to (complex) path mangling (see https://github.com/robn/AROS/commits/posixc-openat for a work-in-progress implementation of openat on top of this).

Open considerations:

  • naming. *Relative is a lot to type, but accurate. *At might be better. I don't particularly care; strong opinions welcome.
  • ABI. I've done the dumbest thing possible and just added LVOs, but I'm aware that these might clash with AmigaOS/MorphOS in the future. @Kalamatee suggested the technique of making an alternative interface via secondary base, but I can't quite see the setup in my head yet. Guidance welcome!

robn added 8 commits April 28, 2021 12:15
With a little shuffling of shared functions
This is really just fiddling around the edges. I don't love that heap
allocation there, but eh, its a rarely-hit codepath anyway. At least now
its tidier and out of the way.
These all just wrap getpacketinfo in a nice easy dumb way.
@jonx

jonx commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

I wanted to see this land, so I rebased the branch onto current master and verified it. A rebased copy is at jonx:dos-lock-relative-rebased (https://github.com/jonx/AROS/tree/dos-lock-relative-rebased) if it's useful to fast-forward this PR onto.

The branch turned out to be in good shape:

  • The only conflict against master was one hunk in getdeviceproc.c: master gained a multidirectory-assign fix (deviceproc_nextassignlock, skipping consecutive entries with the same lock) in the in-place internal that this PR had moved into getdeviceprocrelative.c. I kept the thin GetDeviceProc wrapper and ported that fix into deviceproc_internal in the relative file. Everything else applied clean.
  • Dropped one duplicate: the PR added a second ResolveSoftlink prototype in dos_intern.h (there was already one). Removed the new one.
  • All 8 commits keep their original authorship.

Verified on a darwin/aarch64 hosted build: dos.library compiles clean and the system boots without regression. Since Lock, Open and GetDeviceProc now delegate to their *Relative internals, the boot path itself exercises the refactor (mounting SYS: goes through GetDeviceProc -> GetDeviceProcRelative, the function the conflict touched). I did not run the dedicated lockrelative test, as this host halts at the display-driver check before a console is available.

On the two open considerations from the description: the 12 new LVOs are appended at the tail of dos.conf marked "AROS-specific", which matches how the existing AROS-only calls (ParsePatternNoCase, SameDevice, ScanVars, GetSegListInfo, AssignAddToList) are already carried, so the additive ABI approach looks consistent with current practice. Naming and the secondary-base question are still maintainer calls.

@jonx

jonx commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Worth flagging a tradeoff before anyone reimplements this as a support library: the current code is deliberately race-free. LockRelative/OpenRelative talk to the filesystem handler directly with the base lock (fl_Task packets + the internal ResolveSoftlink), so there is no intermediate path assembly and no TOCTOU window on a retargeted symlink, which was the stated motivation.

A support library limited to the public dos API cannot preserve that: relative-to-lock there means either CurrentDir(lock)+Open (mutates the process CWD, not reentrant) or NameFromLock+AddPart+Open (reassembles a path, reintroducing the symlink race). Both give weaker semantics than the LVO version.

So it seems like a design call: (a) a convenience wrapper in arossupport over the public API, accepting non-atomic relative ops, or (b) keep the race-free handler-level implementation but move it off the primary dos LVO table via a secondary base / internal-linked helper. Happy to do the legwork on whichever you would prefer -- which semantics do you want these to have?

@jonx

jonx commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

You're right, I had that backwards. The atomicity is in the handler's ACTION_LOCATE_OBJECT (the name is resolved against the parent lock in a single packet), and that is fully reachable from the public API: DoPkt to the lock's fl_Task, or GetDeviceProc for the device case, with the standard ACTION_* actions for the rest. The internal fs_LocateObject/dopacket3 wrappers robn used are just conveniences for code living inside dos, not a capability only dos has, so a supplementary shared library gets the same race-free behaviour with the official dos API left untouched. Agreed it shouldn't be bolted into dos.

I'm happy to build that: a separate dosrelative.library exposing the relative set, each function doing GetDeviceProc / fl_Task then DoPkt(ACTION_LOCATE_OBJECT / MAKE_LINK / RENAME_OBJECT / ...) directly, i.e. robn's logic re-homed onto public DoPkt. The name is just a placeholder that says what it does; happy to use whatever you'd prefer. Sound like the right shape before I start?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants