Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Date: Mon, 6 Mar 2017 20:56:14 +0000
From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: dev@subversion.apache.org, Stefan Fuhrmann <stefan2@apache.org>
Cc: commits@subversion.apache.org
Subject: Re: svn commit: r1785754 - in /subversion/trunk/subversion:
 include/svn_error_codes.h libsvn_fs_fs/cached_data.c
 libsvn_fs_fs/cached_data.h libsvn_fs_fs/transaction.c
 tests/libsvn_fs/fs-test.c
Message-ID: <20170306205614.GB5047@fujitsu.shahaf.local2>
References: <20170306195355.C7E213A05B4@svn01-us-west.apache.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170306195355.C7E213A05B4@svn01-us-west.apache.org>
User-Agent: Mutt/1.5.23 (2014-03-12)
archived-at: Mon, 06 Mar 2017 21:01:09 -0000

stefan2@apache.org wrote on Mon, Mar 06, 2017 at 19:53:55 -0000:
> Author: stefan2
> Date: Mon Mar  6 19:53:55 2017
> New Revision: 1785754
> 
> URL: http://svn.apache.org/viewvc?rev=1785754&view=rev
> Log:
> Make FSFS consistency no longer depend on hash algorithms.

First of all, thanks for fixing this.

I didn't do a full review with out-of-diff context, but I did spot two
issues:

> +++ subversion/trunk/subversion/libsvn_fs_fs/transaction.c Mon Mar  6 19:53:55 2017
> @@ -2385,6 +2392,78 @@ get_shared_rep(representation_t **old_re
> +      /* The existing representation may itsel be part of the current

Typo: "itsel".

> +      /* Compare the two representations.
> +       * Note that the stream comparison might also produce MD5 checksum
> +       * errors or other failures in case of SHA1 collisions. */
> +      SVN_ERR(svn_fs_fs__get_contents_from_file(&contents, fs, rep, file,
> +                                                offset, scratch_pool));
> +      SVN_ERR(svn_fs_fs__get_contents(&old_contents, fs, &old_rep_norm,
> +                                      FALSE, scratch_pool));
> +      err = svn_stream_contents_same2(&same, contents, old_contents,
> +                                      scratch_pool);
> +
> +      /* Restore FILE's read / write position. */
> +      SVN_ERR(svn_io_file_seek(file, APR_SET, &old_position, scratch_pool));

Leaks 'err'.

Cheers,

Daniel