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

Re: [Libguestfs] [PATCH] Replace File::Path's remove_tree



On Wed, Jan 26, 2011 at 05:39:06PM +0000, Matthew Booth wrote:
> We were using remove_tree in the RHEV cleanup code. remove_tree seems to use
> chdir. Unfortunately, as this code will be running seteuid(36), it is not
> unlikely that the current working directory will not be readable. This causes
> remove_tree to fail.
> 
> This patch replaces remove_tree with a simple rm -rf. We also only warn on
> failure, as it's not strictly fatal.
> 
> Fixes RHBZ#670778
> ---
>  lib/Sys/VirtV2V/Connection/RHEVTarget.pm |   24 +++++-------------------
>  1 files changed, 5 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/Sys/VirtV2V/Connection/RHEVTarget.pm b/lib/Sys/VirtV2V/Connection/RHEVTarget.pm
> index 7ea84f8..944c324 100644
> --- a/lib/Sys/VirtV2V/Connection/RHEVTarget.pm
> +++ b/lib/Sys/VirtV2V/Connection/RHEVTarget.pm
> @@ -243,7 +243,6 @@ sub DESTROY
>  
>  package Sys::VirtV2V::Connection::RHEVTarget::Vol;
>  
> -use File::Path qw(remove_tree);
>  use File::Spec::Functions;
>  use File::Temp qw(tempdir);
>  use POSIX;
> @@ -380,25 +379,12 @@ sub _cleanup
>  
>      return unless (defined($tmpdir));
>  
> -    my $errors = [];
> -    eval {
> -        remove_tree($tmpdir, { error => \$errors });
> -    };
> -    push(@$errors, $@) if ($@);
> -
> -    if (@$errors > 0) {
> -        foreach my $error (@$errors) {
> -            foreach my $file (keys(%$error)) {
> -                warn(user_message(__x("Error removing {file}: {error}",
> -                                      file => $file,
> -                                      error => $error->{$file})));
> -            }
> -        }
> -
> -        die(user_message(__x("Unable to remove temporary directory ".
> -                             "{dir}", dir => $tmpdir)));
> +    my $ret = system('rm', '-rf', $tmpdir);
> +    if (WEXITSTATUS($ret) != 0) {
> +        warn user_message(__x("Error whilst attempting to remove temporary ".
> +                              "directory {dir}",
> +                              dir => $tmpdir));
>      }
> -
>      $tmpdir = undef;
>  }
>  

Looks better to me, so ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw


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