diff options
author | Ben Pfaff <pfaffben@debian.org> | 2020-11-10 09:42:58 -0500 |
---|---|---|
committer | Zack Weinberg <zackw@panix.com> | 2020-11-10 09:42:58 -0500 |
commit | 1725c947144d9bebfe7817c2c5f0d53d884b1297 (patch) | |
tree | a716ad63e2d39c1768fdea2b7ec8e002dd591cbc | |
parent | 29df8097dd854707c2b456f02f05d2995340c351 (diff) | |
download | autoconf-1725c947144d9bebfe7817c2c5f0d53d884b1297.tar.gz |
autom4te: replace output file atomically (#110305)
In 2003, Joey Hess reported the following bug against Debian's
autoconf package (see http://bugs.debian.org/221483):
I noticed that if I ctrl-c autoconf, it can leave a partially
written, executable configure script. I was lucky enough to
get a configure script that exited with a shell parse error,
but if I had been unlucky, it might have exited 0 without
doing all the tests I expected it to do. That would have
sucked to ship to users.
There are many ways to update a file in a way that is not
prone to these problems, and I suggest that autoconf adopt
one of them.
Ben Pfaff wrote a patch to make autom4te replace the output file
atomically; Debian has carried it since 2006. He submitted it
to autoconf upstream in 2008 but it never went anywhere.
I (Zack) have dusted off the patch and made some minor improvements:
using File::Temp (with DIR set to the directory of the output file)
instead of a predictable temporary file name, and using
Autom4te::FileUtils::update_file instead of File::Copy::move.
I do not attempt to test the fix (the test would be inherently racey)
nor do I have autom4te delete the temp file if it crashes while the
file is being written (there is no way to do this with 100%
reliability and it strikes me as likely to cause more problems than it
solves).
Fixes our bug #110305.
* bin/autom4te.in (handle_output): When $output is to a regular or
nonexistent file, write to a temporary file in the same directory
and then rename it over $output after completion.
-rw-r--r-- | bin/autom4te.in | 40 |
1 files changed, 30 insertions, 10 deletions
diff --git a/bin/autom4te.in b/bin/autom4te.in index 20ecca9e..9aa0d1f1 100644 --- a/bin/autom4te.in +++ b/bin/autom4te.in @@ -546,21 +546,38 @@ sub handle_output ($$) foreach (sort keys %forbidden); verb "allowed tokens: $allowed"; - # Read the (cached) raw M4 output, produce the actual result. We - # have to use the 2nd arg to have Autom4te::XFile honor the third, but then - # stdout is to be handled by hand :(. Don't use fdopen as it means - # we will close STDOUT, which we already do in END. - my $out = new Autom4te::XFile; + # Read the (cached) raw M4 output, produce the actual result. + # If we are writing to a regular file, replace it atomically. + my $atomic_replace = 0; + my $out; if ($output eq '-') { - $out->open (">$output"); + # Don't just make $out be STDOUT, because then we would close STDOUT, + # which we already do in END. + $out = new Autom4te::XFile ('>&STDOUT'); + } + elsif (-e $output && ! -f $output) + { + $out = new Autom4te::XFile ($output, '>'); } else { - $out->open ($output, O_CREAT | O_WRONLY | O_TRUNC, oct ($mode)); + my (undef, $outdir, undef) = fileparse ($output); + + use File::Temp (); + $out = new File::Temp (UNLINK => 0, DIR => $outdir); + fatal "cannot create a file in $outdir: $!" + unless $out; + + # File::Temp doesn't give us access to 3-arg open(2), unfortunately. + # In older Perls, implicit conversion of a File::Temp to its filename + # cannot be relied upon. + chmod (oct ($mode) & ~(umask), $out->filename) + or fatal "setting mode of " . $out->filename . ": $!"; + + $atomic_replace = 1; } - fatal "cannot create $output: $!" - unless $out; + my $in = new Autom4te::XFile ($ocache . $req->id, "<"); my %prohibited; @@ -585,7 +602,8 @@ sub handle_output ($$) foreach (split (/\W+/)) { $prohibited{$_} = $. - if !/^$/ && /$forbidden/o && !/$allowed/o && ! exists $prohibited{$_}; + if !/^$/ && /$forbidden/o && !/$allowed/o + && ! exists $prohibited{$_}; } # Performed *last*: the empty quadrigraph. @@ -595,6 +613,8 @@ sub handle_output ($$) } $out->close(); + update_file ($out->filename, $output, $force) + if $atomic_replace; # If no forbidden words, we're done. return |