Re: [PATCH 17/18] Place the open() call inside the do{} struct and prevent failing close

[Junio C Hamano <gitster@pobox.com>]

Célestin Matte <celestin.matte@ensimag.fr> writes:

> Placing the open() call inside the do{} struct will automatically close the
> filehandle if possible.
> Placing the close() call outside the do{} struct is useless and will make it
> fail systematically
> Change the error message to state that what fails is a fork(), not a file
> opening.
> Use autodie to properly exit when a print or open call fails.
>
> Signed-off-by: Célestin Matte <celestin.matte@ensimag.fr>
> Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
> ---
>  contrib/mw-to-git/git-remote-mediawiki.perl |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
> index 952ddcc..20ddccb 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
> @@ -23,6 +23,7 @@ binmode STDOUT, ':encoding(UTF-8)';
>  
>  use URI::Escape;
>  use Readonly;
> +use autodie;
>  
>  # Mediawiki filenames can contain forward slashes. This variable decides by which pattern they should be replaced
>  Readonly my $SLASH_REPLACEMENT => '%2F';
> @@ -363,8 +364,6 @@ sub run_git {
>  		local $/ = undef;
>  		<$git>
>  	};
> -	close($git);
> -
>  	return $res;
>  }

Confused.  Which part of this patch moves open inside a do{} block?
This was last touched by [9/18] but it doesn't do any such thing,
either.

Upon leaving this subroutine, $git filehandle will go out of scope,
so in that sense, close may not be necessary, but that does not
match what the proposed log message claims what the patch does.

Also, this patch does not remove "or die" 9/18 added, even though
the proposed log message claims that with autodie it is no longer
necessary.

I am not convinced that using autodie globally, without vetting the
calls the original code make, is a good idea in the first place.
How does this change interact with existing calls to open, close,
etc. that check the return value from them, now these calls throw
exception and will not give a chance for the existing error handling
codepath to intervene?