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

Unchecked close of write handles (Was: Re: [SCM] dpkg's main repository branch, master, updated. 1.16.10-146-ga62fb6a)



On 2013-04-24 01:28, Guillem Jover wrote:
> The following commit has been merged in the master branch:
> commit a62fb6adecfcb362e5f8927db33f32676668984a
> Author: Guillem Jover <guillem@debian.org>
> Date:   Sun Dec 30 01:42:11 2012 +0100
> 
>     Use three-argument form of open in perl code
>     
>     Fixes InputOutput::ProhibitTwoArgOpen.
>     
>     Warned-by: perlcritic
> 

Hi,

While reviewing this commit, I noticed a number of cases where write
handles were closed but the return value was not checked.  They appear
to be unrelated to the changes in the commit (i.e. the problem was there
prior to the commit).

~Niels

> [...]
> @@ -66,7 +68,7 @@ sub store_config {
>    # Check that config is completed
>    return if not $config{'done'};
>  
> -  open(VARS, ">$vars") || die "Couldn't open $vars in write mode : $!\n";
> +  open(VARS, '>', $vars) || die "Couldn't open $vars in write mode : $!\n";
>    print VARS Dumper(\%config);
>    close VARS;
     ^^^^^^^^^^^

>  }
> [...]
> @@ -623,7 +623,7 @@ foreach (keys %md5sums) {
>    next if (-f $_);
>    delete $md5sums{$_};
>  }
> -open(MD5SUMS, ">$methdir/md5sums") ||
> +open(MD5SUMS, '>', "$methdir/md5sums") ||
>    die "Can't open $methdir/md5sums in write mode : $!\n";
>  print MD5SUMS Dumper(\%md5sums);
>  close MD5SUMS;
   ^^^^^^^^^^^^^^
> [...]
> diff --git a/src/t/100_dpkg_divert.t b/src/t/100_dpkg_divert.t
> index 0fdf4bc..114a184 100644
> --- a/src/t/100_dpkg_divert.t
> +++ b/src/t/100_dpkg_divert.t
> @@ -48,20 +48,20 @@ sub cleanup {
>  
>  sub install_diversions {
>      my ($txt) = @_;
> -    open(O, "> $admindir/diversions");
> +    open(O, '>', "$admindir/diversions");
>      print O $txt;
>      close(O);
       ^^^^^^^^
>  }
>  
>  sub install_filelist {
>      my ($pkg, $arch, @files) = @_;
> -    open(L, "> $admindir/info/$pkg.list");
> +    open(L, '>', "$admindir/info/$pkg.list");
>      for my $file (@files) {
>          print L "$file\n";
>      }
>      close(L);
       ^^^^^^^^
>      # Only installed packages have their files list considered.
> [...]
> 


Reply to: