Skip to content

[Sketch] openat, fsync, and sync#43

Draft
milseman wants to merge 1 commit into
apple:mainfrom
milseman:open_at_sync
Draft

[Sketch] openat, fsync, and sync#43
milseman wants to merge 1 commit into
apple:mainfrom
milseman:open_at_sync

Conversation

@milseman

Copy link
Copy Markdown
Contributor

No description provided.

@milseman milseman mentioned this pull request Mar 22, 2021
@milseman

milseman commented Oct 3, 2021

Copy link
Copy Markdown
Contributor Author

This is close to being mergeable, but still has the global system call design question as #33

@lorentey lorentey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 🚢

///
/// - Parameters:
/// - path: The location of the file to open.
/// - at: if `path` is relative, treat it as relative to this file descriptor

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: we should not use prepositions as variable names.

Suggested change
/// - at: if `path` is relative, treat it as relative to this file descriptor
/// - base: if `path` is relative, treat it as relative to this file descriptor

Or we could just call it fd.

@_alwaysEmitIntoClient
public static func open(
_ path: FilePath,
at: FileDescriptor,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
at: FileDescriptor,
at base: FileDescriptor,

(etc, throughout this PR.)

@rauhul

rauhul commented Nov 14, 2021

Copy link
Copy Markdown
Contributor

@milseman @lorentey Do we want this to be called open(_:at:...) or open(_:relativeTo:...). Whatever precedent is set here should be followed by my fstatat functions

@lorentey

Copy link
Copy Markdown
Member

I'd be happy with either option! I like relativeTo:, too.

@milseman

milseman commented Nov 15, 2021

Copy link
Copy Markdown
Contributor Author

I much prefer relativeTo: personally as well, as I otherwise would do a double-take in the man page or docs to make sure at means what it means in openat(2).

edit: Oh right, that is ignored if the incoming path is absolute. FilePath has a nice API for relative and absolute paths, but I don't think we should go overboard here. We could assert/precondition the path is relative, silently pass on to openat, etc. Thoughts?

edit 2: The existence of AT_FDCWD makes me think we should give a teeny tiny bit of API polish here

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.

3 participants