Re: [PATCH] Clean up temporary file in hardlink mode

[Tom de Vries <tdevries@suse.de>]

On 02-03-19 15:48, Tom de Vries wrote:
> Hi,
> 
> Consider an executable file with hardlinks a.out and b.out.
> 
> When running dwz once:
> ...
> $ dwz -h a.out b.out
> ...
> a.out and b.out are updated, and remain hardlinks to the same file.
> 
> But when running dwz once more, a.out and b.out remain unchanged, and a
> temporary file b.out.#dwz#.XXXXXX is left.
> 
> This is caused by the fact that the code in function dwz intended to handle
> unchanged hardlinks is never triggered.  It is guarded by a "resa[n].res == 1"
> condition, but res->res is set to 0 at the end of function dwz, irrespective
> of whether the file changed or not.
> 
> Fix this by only setting res->res to 0 if the file changed.
> 

To put it in terms of the --trace switch I just posted, the effect of
the patch is:
...
 $ dwz -t -h a.out b.out
 Compressing a.out
 Updating hardlink b.out to changed file
 $ dwz -t -h a.out b.out
 Compressing a.out
 dwz: a.out: compression not beneficial - old size 3444 new size 3444
-Updating hardlink b.out to changed file
+Skipping hardlink b.out to unchanged file
...

Thanks,
- Tom

> OK for trunk?
> 
> [ Applies on top of trunk + "[PATCH] Don't process low-mem files in multifile
> mode". ]
> 
> Thanks,
> - Tom
> 
> Clean up temporary file in hardlink mode
> 
> 2019-03-02  Tom de Vries  <tdevries@suse.de>
> 
> 	PR dwz/24275
> 	* dwz.c (struct file_result): Document res field.
> 	(dwz): Only set res->res to 1 if changed.
> 	* testsuite/dwz.tests/hardlink-no-change.sh: New test.
> 
> ---
>  dwz.c                                     |  9 +++++++--
>  testsuite/dwz.tests/hardlink-no-change.sh | 23 +++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/dwz.c b/dwz.c
> index ffa8e08..3673bbf 100644
> --- a/dwz.c
> +++ b/dwz.c
> @@ -10950,6 +10950,11 @@ remove_empty_pus (void)
>  /* Helper structure for hardlink discovery.  */
>  struct file_result
>  {
> +  /* -2: Already processed under different name.
> +     -1: Ignore.
> +      0: Processed, changed.
> +      1: Processed, unchanged.
> +  */
>    int res;
>    dev_t dev;
>    ino_t ino;
> @@ -11206,6 +11211,8 @@ dwz (const char *file, const char *outfile, struct file_result *res,
>  
>  	  if (write_dso (dso, outfile, &st))
>  	    ret = 1;
> +	  else
> +	    res->res = 0;
>  	}
>      }
>  
> @@ -11227,8 +11234,6 @@ dwz (const char *file, const char *outfile, struct file_result *res,
>    close (fd);
>  
>    free (dso);
> -  if (ret == 0)
> -    res->res = 0;
>    return ret;
>  }
>  
> diff --git a/testsuite/dwz.tests/hardlink-no-change.sh b/testsuite/dwz.tests/hardlink-no-change.sh
> new file mode 100755
> index 0000000..945d220
> --- /dev/null
> +++ b/testsuite/dwz.tests/hardlink-no-change.sh
> @@ -0,0 +1,23 @@
> +#!/bin/sh
> +
> +set -e
> +
> +cp ../hello 1
> +ln 1 2
> +
> +dwz -h 1 2
> +
> +dwz -h 1 2 2>/dev/null
> +
> +smaller-than.sh 1 ../hello
> +smaller-than.sh 2 ../hello
> +
> +hl="$(find -samefile 1)"
> +hl="$(echo $hl)"
> +[ "$hl" = "./1 ./2" ]
> +
> +ls=$(ls)
> +ls=$(echo $ls)
> +[ "$ls" = "1 2" ]
> +
> +rm -f 1 2
>