Page MenuHomeFreeBSD

open.2: Editorial pass
ClosedPublic

Authored by jhb on Mon, Dec 30, 4:23 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jan 3, 10:42 PM
Unknown Object (File)
Fri, Jan 3, 4:39 PM
Unknown Object (File)
Fri, Jan 3, 11:58 AM
Unknown Object (File)
Thu, Jan 2, 6:04 PM
Unknown Object (File)
Tue, Dec 31, 4:40 AM
Subscribers

Details

Summary
  • Use a typical tagged list for the open flags instead of a literal block. This permits using markup in the flag descriptions. Also, drop the offset to avoid indenting the entire list.
  • Note that O_RESOLVE_BENEATH only applies to openat(2)
  • Use a clearer description of O_CLOEXEC (what it means, not the internal flag it sets)
  • Note that exactly one permission flag is required.
  • Split up a paragraph on various flags so that each flag gets its own paragraph. Some flags already had their own paragraph, so this is more consistent. It also makes it clearer which flag a sentence is talking about when a flag has more than one sentence.
  • Appease some errors from igor and man2ps
  • In the discussion about a returned directory descriptor opened with O_SEARCH, avoid the use of Fa fd since the descriptor in question is a return value and not an argument to open or openat.
  • Various and sundry markup and language tweaks

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jhb requested review of this revision.Mon, Dec 30, 4:23 PM

It seems a number of existing O_* descriptions aren't great, if you want to tackle some of them at the same time. Otherwise if you want to improve the formatting now and maybe come back to descriptions that's fine with me.

lib/libsys/open.2
155

Maybe the alias comment should be a parenthetical?

162

Never looked at this before to notice, but "append on each write" seems odd (it seems to suggest writes overwrite, if O_APPEND not specified).

168

this could be grammatically better

174

this seems insufficiently specific

178

perhaps we should indicate O_SYNC / O_DSYNC relationship / superset

194

verify how?

369
427–429

or "and the O_EMPTY_PATH flag"

460

We can maybe comment more on how this relates to other operating systems

472

or "and the description of the O_CLOEXEC flag"

This revision is now accepted and ready to land.Mon, Dec 30, 4:56 PM
lib/libsys/open.2
162

I mean suggests each write overwrites w/o O_APPEND

jhb marked 6 inline comments as done.Mon, Dec 30, 5:38 PM
jhb added inline comments.
lib/libsys/open.2
162

I think it's clear. Other writes might overwrite if you are using lseek(2) or pwrite(2), but O_APPEND prevents that. I think O_APPEND also means you can't seek out beyond EOF and write either which creates a hole normally. (Yes, confirmed. Most filesystems just set uio_offset to EOF before doing the write which means that the file position gets reset to the new EOF after each write as well even if you've used lseek(2) or pwrite(2)).

Maybe, "append on each write instead of writing at the current file pointer"?

174

I think the semantic is also somewhat poorly defined. I want to say it means "read or write directly from the backing store discarding any cached data"?

178

I've added "metadata and data writes"

194

This is for MAC_VERIEXEC I think. Juniper added this and I think it's hard to explain this without a paragraph so am tempted to leave it as is?

460

Hmm, I'm not sure what the rule on other OS's is.

lib/libsys/open.2
194

how about "verify the contents of the file using VERIEXEC(4)" or "using a MAC(3) policy such as VERIEXEC(4)"
Any reference to MAC(3) and/or VERIEXEC(4) is fine I'd think.

lib/libsys/open.2
162

SGTM if there's enough room.

The Linux man page says "Before each write(2), the file offset is positioned at the end of the file, as if with lseek(2)."
Opengroup "If set, the file offset shall be set to the end of the file prior to each write."

If we want something short, maybe "set file offset to end of file before each write"

jhb marked an inline comment as done.

More tweaks from Ed

This revision now requires review to proceed.Mon, Dec 30, 6:30 PM
emaste added inline comments.
lib/libsys/open.2
156

Should this use .Pq/.Po/.Pc? (I'm not sure when that's appropriate/required.)

This revision is now accepted and ready to land.Mon, Dec 30, 6:37 PM
jhb marked an inline comment as done.Mon, Dec 30, 6:40 PM
jhb added a subscriber: kib.
jhb added inline comments.
lib/libsys/open.2
174

I guess the other weird thing here is that I think reads might flush cached write data as a side effect? Maybe @kib can confirm what the semantics are?

This revision now requires review to proceed.Mon, Dec 30, 6:41 PM
lib/libsys/open.2
460

I recall this coming up recently somehow, and I thought it was in the context of open() flags for linuxulator but it seems that wasn't it.

https://forums.freebsd.org/threads/sgid-on-directories.70392/ discusses the different behaviours -- as this isn't related to open() flags no need for a reference here.

lib/libsys/open.2
116

I think it would be very useful to define the meaning of 'strictly relative'.

174

This is about O_DIRECT?

For FFS: on read, flush both dirty buffers and dirty pages for the read range, then read directly from underlying device into user buffer. On write, do not delay writes (but do not wait for write to complete).

For tmpfs: ignored, semantically all reads and writes are O_DIRECT since double-copy was eliminated.

For msdosfs: silently ignored.

For ZFS: no idea.

225–227

causes each write on the resulting file descriptor
to be appended to the end of the file.

446

I believe it is useful to note there that the allocated file descriptor is the lowest unused fd number at the moment of the call.

lib/libsys/open.2
116

I wonder if that is what the next sentence tries to define? At least, the next sentence is what I think is meant by this phrase.

156

I'm not sure either. We have a mix in the tree. I think I used Pq below to try to ensure the closing ) is marked up correctly, but probably just using ()'s is ok even below. Dq/Do/Dc give typographic double-quotes that aren't present in ASCII, but Pq/Po/Pc aren't the same there.

174

I think NFS also has a notion of O_DIRECT that is akin to FFS. I thought that writes on FFS (and NFS) effectively invalidated any clean buffers for the range being written?

446

Yes, this is even in POSIX's description

lib/libsys/open.2
116

I mean, it should be started by 'i.e.' or some other continuing hint. Otherwise the sentence is read as an additional requirement, instead of the term definition IMO.

174

Right, NFS is also r/w. There, both read and write are direct RPC from server into user buffer. I.e. there is no cache consistency at all. Normally IO_DIRECT is ignored, a sysctl needs to be set for it to behave this way.

jhb marked 8 inline comments as done.Thu, Jan 2, 6:53 PM
jhb added inline comments.
lib/libsys/open.2
174

There is a paragraph for O_DIRECT further below that I think is probably ok. I've added a note there about the fact that it is filesystem dependent. I have altered the description here though to use the phrase I mentioned above ("direct access to backing store") as I think it is a more intuitive description for the flag name.

446

I reused the language from dup(2) here.

More tweaks from Konstantin

This revision is now accepted and ready to land.Thu, Jan 2, 6:59 PM
kib added inline comments.
lib/libsys/open.2
303
jhb marked an inline comment as done.Fri, Jan 3, 3:48 PM
This revision was automatically updated to reflect the committed changes.