autoconf-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: replace autom4te output file atomically


From: Ralf Wildenhues
Subject: Re: replace autom4te output file atomically
Date: Tue, 12 Aug 2008 07:56:33 +0200
User-agent: Mutt/1.5.18 (2008-07-21)

Hi Ben,

Thanks for passing the patch upstream.  Sorry for finding issues with it
now.

* Ben Pfaff wrote on Tue, Aug 12, 2008 at 07:14:09AM CEST:

> diff --git a/bin/autom4te.in b/bin/autom4te.in
> old mode 100644
> new mode 100755

Why this mode change?

> index 685df41..1ea86da
> --- a/bin/autom4te.in
> +++ b/bin/autom4te.in

> @@ -550,21 +550,36 @@ sub handle_output ($$)

>    else
>      {
> -      $out->open($output, O_CREAT | O_WRONLY | O_TRUNC, oct ($mode));
> +      $out->open ("$output.tmp", O_CREAT | O_WRONLY | O_TRUNC, oct ($mode));

Hmm.  This may still leave us competing with a concurrent autoconf
running.  I'm not saying that this is something we need (there are
more places that need fixing in order to guard against this), but
shouldn't a temporary file have a temp name (or at least using 
a PID or so)?

We use mktmpdir earlier, but possibly on a different mount, which
will break the atomicity if we create the file there.

> @@ -600,6 +615,12 @@ sub handle_output ($$)
>  
>    $out->close();
>  
> +  if ($atomic_replace && !rename ("$output.tmp", "$output"))
> +    {
> +      move ("${output}.tmp", "$output")

Please use a variable for "${output}.tmp".

> +     or fatal "cannot rename ${output}.tmp as $output: $!";

How about still trying to remove the temp file?

A test case to expose the issue fixed by this patch would be nice
(but I'm not sure how to do it easily).

Thanks,
Ralf




reply via email to

[Prev in Thread] Current Thread [Next in Thread]