~sircmpwn/hare-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
5 3

[PATCH hare v2] fs,os: add copy()

Details
Message ID
<20230513000811.12361-1-autumnull@posteo.net>
DKIM signature
pass
Download raw message
Patch: +28 -0
Signed-off-by: Autumn! <autumnull@posteo.net>
---
accidentally did plagiarism badly
 fs/fs.ha | 23 +++++++++++++++++++++++
 os/os.ha |  5 +++++
 2 files changed, 28 insertions(+)

diff --git a/fs/fs.ha b/fs/fs.ha
index af630261..c7335bb7 100644
--- a/fs/fs.ha
+++ b/fs/fs.ha
@@ -102,6 +102,29 @@ export fn rename(fs: *fs, oldpath: str, newpath: str) (void | error) = {
	};
};

// Copy a file from oldpath to newpath. Preserves the file permissions.
export fn copy(fs: *fs, oldpath: str, newpath: str) (void | error) = {
	// TODO: copy non-regular files
	let st = stat(fs, oldpath)?;
	assert(isfile(st.mode), "TODO: copy non-regular files");
	let old = open(fs, oldpath)?;
	let new = match (create(fs, newpath, st.mode)) {
	case let h: io::handle =>
		yield h;
	case let err: error =>
		io::close(old): void;
		return err;
	};
	match (io::copy(new, old)) {
	case let err: io::error =>
		io::close(new): void;
		io::close(old): void;
		remove(fs, newpath)?;
		return err;
	case size => void;
	};
};

// Moves a file. This will use [[rename]] if possible, and will fall back to
// copy and remove if necessary.
export fn move(fs: *fs, oldpath: str, newpath: str) (void | error) = {
diff --git a/os/os.ha b/os/os.ha
index 69e6ae77..4cb76275 100644
--- a/os/os.ha
+++ b/os/os.ha
@@ -19,6 +19,11 @@ export fn remove(path: str) (void | fs::error) = fs::remove(cwd, path);
export fn rename(oldpath: str, newpath: str) (void | fs::error) =
	fs::rename(cwd, oldpath, newpath);


// Copies a file from oldpath to newpath. Preserves the permissions.
export fn copy(oldpath: str, newpath: str) (void | fs::error) =
	fs::copy(cwd, oldpath, newpath);

// Moves a file. This will use [[rename]] if possible, and will fall back to
// copy and remove if necessary.
export fn move(oldpath: str, newpath: str) (void | fs::error) =
-- 
2.40.1

[hare/patches] build success

builds.sr.ht <builds@sr.ht>
Details
Message ID
<CSKPPFMLKWEF.XG4JWMPO2CFZ@cirno2>
In-Reply-To
<20230513000811.12361-1-autumnull@posteo.net> (view parent)
DKIM signature
missing
Download raw message
hare/patches: SUCCESS in 1m42s

[fs,os: add copy()][0] v2 from [Autumn!][1]

[0]: https://lists.sr.ht/~sircmpwn/hare-dev/patches/41097
[1]: autumnull@posteo.net

✓ #988948 SUCCESS hare/patches/alpine.yml  https://builds.sr.ht/~sircmpwn/job/988948
✓ #988949 SUCCESS hare/patches/freebsd.yml https://builds.sr.ht/~sircmpwn/job/988949
Details
Message ID
<CSKZMQG0SG3T.34KKWGF3403PF@gura>
In-Reply-To
<20230513000811.12361-1-autumnull@posteo.net> (view parent)
DKIM signature
pass
Download raw message
TOCTOU. Also maybe out of scope?
Details
Message ID
<c6719eed-9056-4a87-b7c9-6c21b55c8d41@posteo.net>
In-Reply-To
<CSKZMQG0SG3T.34KKWGF3403PF@gura> (view parent)
DKIM signature
pass
Download raw message
> TOCTOU. Also maybe out of scope?
imho not out of scope, or at least, not moreso than move(). but yeah 
this patch is a little scruffy because i needed it quickly for the build 
driver and didn't want to get too deep in the weeds so i just copied the 
existing copying code from move() (as well as its flaws). i'll spend a 
bit longer on it but i do think the function is broadly useful.
Details
Message ID
<a740ecb1-7f89-cdda-c385-7ad34e7c0e62@posteo.net>
In-Reply-To
<c6719eed-9056-4a87-b7c9-6c21b55c8d41@posteo.net> (view parent)
DKIM signature
pass
Download raw message
> TOCTOU. Also maybe out of scope?
having put more thought into this, i'm not sure if this is actually 
buggy behavior, since the permissions are not actually used until after 
the file is successfully opened, and they're just copied across to the 
new file. i don't really see where the potential unexpected behavior 
lies here. ~illiliti's point about the permissions being filtered 
through umask is relevant though and can be fixed i think.

alternatively, if there is in fact a bug here, fixing this will require 
an uncomfortably large refactor of fs:: and io::, since fstat would 
probably want to be io::stat since it deals with file descriptors, but 
it relies on fs::filestat, which can't be used by io since fs relies on 
io already, so it might need to move to io::filestat which feels wrong. 
there's a big tangled can of worms here that i don't want to get into tbqh.

~Autumn
Details
Message ID
<CSLVL1PTYPMM.2JD9AEU38DBF5@taiga>
In-Reply-To
<c6719eed-9056-4a87-b7c9-6c21b55c8d41@posteo.net> (view parent)
DKIM signature
pass
Download raw message
Okay, in scope. If you can find a TOCTOU approach that'd be nice but if
not then no worries, make sure to document the edge case.
Reply to thread Export thread (mbox)