Clarify that copy_file_range(2) only operates on regular file objects in copy_file_range.2
ClosedPublic
Actions

Authored by rmacklem on Dec 28 2023, 11:32 PM.

Details

Reviewers

karels
pauamma_gundo.com

Group Reviewers

manpages

Commits

rG84b4342c0d7a: copy_file_range.2: Clarify that only regular files work

Summary

PR#273962 reported that copy_file_range(2) did not work
on shared memory objects and returned EINVAL.
Although the reporter felt this was incorrect, it is what
the Linux copy_file_range(2) syscall does.

Since there was no collective agreement that the FreeBSD
semantics should be changed to no longer be Linux compatible,
copy_file_range(2) still works on regular files only.

This man page update clarifies that. If, someday, copy_file_range(2)
is changed to support non-regular files, then the man page will
need to be updated to reflect that.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rmacklem created this revision.Dec 28 2023, 11:32 PM

Herald added a subscriber: imp. · View Herald TranscriptDec 28 2023, 11:32 PM

rmacklem requested review of this revision.Dec 28 2023, 11:32 PM

The change seems reasonable to me. I marked a pre-existing nit that is probably worth fixing while in the area.

lib/libc/sys/copy_file_range.2
185–191	Not your fault, but might as well fix the disagreement of "flags" and "is". Either change "is" to "are", or better yet, change to "The .Fa flags parameter is not zero" (on 3 lines).

Oh, the title has a typo in the man page name.

Ok, I'm not seeing the typo in the man page name?
(Can you please point it out.)

Thanks, rick

In D43227#985196, @rmacklem wrote:

Ok, I'm not seeing the typo in the man page name?

"ramge" instead of "range".

(And s/retruned/returned/ in the summary, while you're at it.)

Something weird is going on...
I searched for rumge (both lower and upper case) and there
is no occurrence of it in the copy_file_range.2 I an diff'ng against.
Same goes for retrun.

Oh, ok, I now see retruned in the Summary here on
phabricator (not in the diff). That I can fix.

I still do not see where RAMGE or ramge is.

Oh, now I see you are talking about the Title here.
I don't know how to change that and it won't affect
the commit.
(I only copy the Summary into the commit and then edit
it from there.)

Changed the description for EINVAL for flags not 0,
as suggested by karels@, except I used "argument"
instead of "parameter", since .FA is defined as function argument.

rmacklem marked an inline comment as done.Dec 29 2023, 1:42 AM

rmacklem retitled this revision from Clarify that copy_file_range(2) only operates on regular file objects in copy_file_ramge.2 to Clarify that copy_file_range(2) only operates on regular file objects in copy_file_range.2.Dec 29 2023, 1:45 AM

rmacklem edited the summary of this revision. (Show Details)

Looks good to me.

This revision is now accepted and ready to land.Dec 29 2023, 2:27 AM

pauamma_gundo.com accepted this revision.Dec 29 2023, 2:56 AM

Closed by commit rG84b4342c0d7a: copy_file_range.2: Clarify that only regular files work (authored by rmacklem). · Explain WhyDec 29 2023, 11:01 PM

This revision was automatically updated to reflect the committed changes.

rmacklem added a commit: rG84b4342c0d7a: copy_file_range.2: Clarify that only regular files work.