HomeFreeBSD

install: Always use a temporary file.

Description

install: Always use a temporary file.

Previously, we would only use a temporary file if explicitly asked to
with the -S option, and even then, only if the target file already
existed. This meant that an outside observer looking for the target
file might see a partial file, and might see the file disappear and
then reappear.

With this patch, we always use a temporary file, ensuring atomicity.
The downside is slightly increased disk usage. The upside is never
having to worry about, for instance, cron jobs randomly failing if
they happen to run simultaneously with make installworld.

The -S option is retained, partly for compatibility, and partly
to control the use of fsync(2), which has a non-negligible cost
(approximately 10% increase in wall time for make installworld).

MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: 0mp, brooks, imp, markj
Differential Revision: https://reviews.freebsd.org/D44742

(cherry picked from commit e5035d08578e372d40b4e2d4c3574b7583549bd6)

install: Simplify path construction.

There's no need to copy the path twice to split it into base and dir.
We simply call basename() first, then handle the two trivial cases in
which it isn't safe to call dirname().

While here, add an early check that the destination is not an empty
string. This would always fail eventually, so it may as well fail
right away. Also add a test case for this shortcut.

MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D44743

(cherry picked from commit 17dc7017d7375b3463d65adffe1eb980b0f86795)

install: Don't skip syncing in the common case.

In copy(), if no digest was requested (which is the common case), we
use copy_file_range() to avoid needlessly copying the contents of the
file into user space and back. When copy_file_range() returns
successfully (which, again, is the common case), we simply return, and
therefore never get to the point where we call fsync() if the -S
option was specified. Fix this.

MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D44756

(cherry picked from commit 4336161cc9c631d40d00adee97dfc8161b6bec9f)

install: Remove the mmap(2) option.

We already removed it from cp(1) over a year ago but never followed up
here. Do so now, for the same reasons: significant complexity for
little to no benefit.

MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D44809

(cherry picked from commit a0439a1b820fa0e742c00d095f5f5c06f5f19432)

install: Assorted nitpickery.

  • Use errc() instead of manually setting errno before calling err().
  • Change one warning into a fatal error.
  • Drop some unnecessary casts.
  • strlcat() bounds checks were off-by-one. This does not matter in practice because the subsequent code renders an overrun harmless.
  • We were passing SSIZE_MAX to copy_file_range() instead of the requested size. This only matters if we're asked to install a file which is still being written to while we are copying it.

MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D44810

(cherry picked from commit 000a533e6d1db9878296b32d1cc212e11a2cc718)

Details

Provenance
desAuthored on Apr 12 2024, 5:30 PM
Reviewer
0mp
Differential Revision
D44742: install: Always use a temporary file.
Parents
rG689dbdedd8bd: heimdal: asn1: Use unsigned bitfields for named bitsets
Branches
Unknown
Tags
Unknown