Re: [PATCH] test-container: Use nftw instead of rm -rf

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2023-09-27 20:20, Adhemerval Zanella wrote:
> If the binary to run is 'env', test-containers skips it and adds
> any required environment variable on the process envs variables.
> This simplifies the required code to spawn new process (no need
> to build an env-like program).
> 
> However, this is an issue for recursive_remove if there is any
> LD_PRELOAD, since test-container will not prepend the loader command
> along with required paths.  If the required preloaded library can
> not be loaded by the system glibc, the 'post-clean rsync' will
> eventually fail.
> 
> One example is if system glibc does not support DT_RELR and the
> built glibc does, the nss/tst-nss-gai-hv2-canonname test fails
> with:
> 
> ../scripts/evaluate-test.sh nss/tst-nss-gai-hv2-canonname $? false false
> 86_64-linux-gnu/nss/tst-nss-gai-hv2-canonname.test-result
> rm: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_ABI_DT_RELR' not
> found (required by x86_64-linux-gnu/malloc/libc_malloc_debug.so)
> 
> Instead trying to figure out the required loader arguments on how
> to spawn the 'rm -rf', replace the command with a nftw call.
> 
> Checked on x86_64-linux-gnu.
> ---

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

>   support/test-container.c | 34 +++++++++++-----------------------
>   1 file changed, 11 insertions(+), 23 deletions(-)
> 
> diff --git a/support/test-container.c b/support/test-container.c
> index 788b091ea0..95dfef1a99 100644
> --- a/support/test-container.c
> +++ b/support/test-container.c
> @@ -37,6 +37,7 @@
>   #include <errno.h>
>   #include <error.h>
>   #include <libc-pointer-arith.h>
> +#include <ftw.h>
>   
>   #ifdef __linux__
>   #include <sys/mount.h>
> @@ -405,32 +406,19 @@ file_exists (char *path)
>     return 0;
>   }
>   
> +static int
> +unlink_cb (const char *fpath, const struct stat *sb, int typeflag,
> +	   struct FTW *ftwbuf)
> +{
> +  return remove (fpath);
> +}
> +
>   static void
>   recursive_remove (char *path)
>   {
> -  pid_t child;
> -  int status;
> -
> -  child = fork ();
> -
> -  switch (child) {
> -  case -1:
> -    perror("fork");
> -    FAIL_EXIT1 ("Unable to fork");
> -  case 0:
> -    /* Child.  */
> -    execlp ("rm", "rm", "-rf", path, NULL);
> -    FAIL_EXIT1 ("exec rm: %m");
> -  default:
> -    /* Parent.  */
> -    waitpid (child, &status, 0);
> -    /* "rm" would have already printed a suitable error message.  */
> -    if (! WIFEXITED (status)
> -	|| WEXITSTATUS (status) != 0)
> -      FAIL_EXIT1 ("exec child returned status: %d", status);
> -
> -    break;
> -  }
> +  int r = nftw (path, unlink_cb, 1000, FTW_DEPTH | FTW_PHYS);
> +  if (r == -1)
> +    FAIL_EXIT1 ("recursive_remove failed");
>   }
>   
>   /* Used for both rsync and the mytest.script "cp" command.  */