[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r907414 - /subversion/trunk/subversion/libsvn_wc/questions.c

From: Greg Stein <gstein_at_gmail.com>
Date: Mon, 8 Feb 2010 14:58:31 -0500

On Sun, Feb 7, 2010 at 06:50, <rhuijben_at_apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/questions.c Sun Feb  7 11:50:50 2010
> @@ -367,12 +367,15 @@
>       return SVN_NO_ERROR;
>     }
>
> +  SVN_ERR(err);
> +
>   /* Check all bytes, and verify checksum if requested. */
> -  SVN_ERR(compare_and_verify(modified_p, db, local_abspath, pristine_stream,
> -                             compare_textbases, force_comparison,
> -                             scratch_pool));
> +  err = compare_and_verify(modified_p, db, local_abspath, pristine_stream,
> +                           compare_textbases, force_comparison,
> +                           scratch_pool);
>
> -  return SVN_NO_ERROR;
> +  return svn_error_compose_create(err,
> +                                  svn_stream_close(pristine_stream));
>  }

It would be better to have compare_and_verify() close the stream after
it has done its work. Invariably, a stream on a file will not be used
once it hits EOF, so you may as well close it instead of waiting for
pool-cleanup.

compare_and_verify() is a local function, so it should be easy enough
to look at all uses, and ensure that closing the stream is acceptable.
If a (semi)public function passes one of its arguments into
compare_and_verify(), then you can use svn_stream_disown() before
passing that stream into compare_and_verify().

If a function that does that is semi-public, then you could even
examine all of its callers, and adjust the docstrings accordingly, to
talk about stream closure.

Cheers,
-g
Received on 2010-02-08 20:59:05 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.