Page MenuHomeFreeBSD

cp: fall back to read/write if copy_file_range fails
ClosedPublic

Authored by asomers on Thu, Sep 10, 6:11 PM.

Details

Summary

cp: fall back to read/write if copy_file_range fails

Even though copy_file_range has a file-system agnostic version, it still
fails on devfs (perhaps because the file descriptor is non-seekable?) In
that case, fallback to old-fashioned read/write. Fixes
"cp /dev/null /tmp/null"

Reported-by: Michael Butler

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

asomers created this revision.Thu, Sep 10, 6:11 PM
asomers requested review of this revision.Thu, Sep 10, 6:11 PM
imp added a comment.Thu, Sep 10, 6:16 PM

Minor nit, which could report a bad write total in some rare circumstances is the only issue I see.

bin/cp/utils.c
249 ↗(On Diff #76892)

if rcount < 0, then this code will execute where before it would not.

asomers added inline comments.Thu, Sep 10, 6:20 PM
bin/cp/utils.c
249 ↗(On Diff #76892)

Yeah, I'm aware of that. But I didn't think inaccurate SIGINFO output in the event of an error was a problem. Do you want me to fix it?

mjg accepted this revision.Fri, Sep 11, 8:27 PM
mjg added a subscriber: mjg.

This is a serious regression, just please get this in. The SIGINFO nit can be sorted out later.

This revision is now accepted and ready to land.Fri, Sep 11, 8:27 PM
mjg added a comment.Fri, Sep 11, 8:39 PM

I think it fails because vn_copy_file_range checks for v_type == VREG while /dev/null is VCHR. There may be other problems.

This revision was automatically updated to reflect the committed changes.
jmg added a subscriber: jmg.Sat, Sep 12, 12:23 AM

Might I suggest that you enable NetBSD's cp tests, and add necessary tests to prevent this breakage again?

allanjude added inline comments.
head/bin/cp/utils.c
245

Is there a reason this isn't juse an else?

imp added inline comments.Sat, Sep 12, 2:16 AM
head/bin/cp/utils.c
245

Think first time through