RE: Problem introduced in revision 34378 of libsvn_fs_fs/fs_fs.c?

I am talking about the test added in commit 34388 of fs_fs.c.

What you see is that the sha1 checksum is written out in
representation_string(...) when the format of the repository is less
then SVN_FS_FS__MIN_REP_SHARING_FORMAT. But I think that the sha1
checksum should be written when the format is higher or equal.

Marc

-----Original Message-----
From: dglasser_at_gmail.com [mailto:dglasser_at_gmail.com] On Behalf Of David
Glasser
Sent: 26 November 2008 18:04
To: Marc Haesen
Cc: dev_at_subversion.tigris.org
Subject: Re: Problem introduced in revision 34378 of
libsvn_fs_fs/fs_fs.c?

I think that Hyrum's original version of this code didn't have a check
at all, which was a change.

ie, you should be comparing 34428 to something rather old, not ot
something as recent at 34377.

--dave

On Wed, Nov 26, 2008 at 1:51 AM, Marc Haesen
<Marc.Haesen_at_oneaccess-net.com> wrote:
> Hi,
>
>
>
> Could it be that the following code changed in revision 34378 of
> libsvn_fs_fs/fs_fs.c:
>
>
>
> ===================================================================
>
> --- fs_fs.c (revision 34377)
>
> +++ fs_fs.c (revision 34428)
>
> @@ -1867,19 +1866,21 @@
>
> }
>
>
>
>
>
> -/* Return a formatted string that represents the location of
>
> - representation REP. If MUTABLE_REP_TRUNCATED is given, the rep is
>
> - for props or dir contents, and only a "-1" revision number will be
>
> - given for a mutable rep. Perform the allocation from POOL. */
>
> +/* Return a formatted string, compatible with filesystem format
FORMAT,
>
> + that represents the location of representation REP. If
>
> + MUTABLE_REP_TRUNCATED is given, the rep is for props or dir
contents,
>
> + and only a "-1" revision number will be given for a mutable rep.
>
> + Perform the allocation from POOL. */
>
> static const char *
>
> representation_string(representation_t *rep,
>
> - svn_boolean_t mutable_rep_truncated, apr_pool_t
> *pool)
>
> + int format,
>
> + svn_boolean_t mutable_rep_truncated,
>
> + apr_pool_t *pool)
>
> {
>
> - /* ###: Make the writing of the sha1 and reuse count dependent on
the
>
> - format number. */
>
> if (rep->txn_id && mutable_rep_truncated)
>
> return "-1";
>
> - else
>
> +
>
> + if (format < SVN_FS_FS__MIN_REP_SHARING_FORMAT)
>
> return apr_psprintf(pool, "%ld %" APR_OFF_T_FMT " %"
SVN_FILESIZE_T_FMT
>
> " %" SVN_FILESIZE_T_FMT " %s %s %"
APR_INT64_T_FMT,
>
> rep->revision, rep->offset, rep->size,
>
>
>
>
>
> Should have been (look at difference in test on repository format):
>
>
>
> ===================================================================
>
> --- fs_fs.c (revision 34377)
>
> +++ fs_fs.c (revision 34428)
>
> @@ -1867,19 +1866,21 @@
>
> }
>
>
>
>
>
> -/* Return a formatted string that represents the location of
>
> - representation REP. If MUTABLE_REP_TRUNCATED is given, the rep is
>
> - for props or dir contents, and only a "-1" revision number will be
>
> - given for a mutable rep. Perform the allocation from POOL. */
>
> +/* Return a formatted string, compatible with filesystem format
FORMAT,
>
> + that represents the location of representation REP. If
>
> + MUTABLE_REP_TRUNCATED is given, the rep is for props or dir
contents,
>
> + and only a "-1" revision number will be given for a mutable rep.
>
> + Perform the allocation from POOL. */
>
> static const char *
>
> representation_string(representation_t *rep,
>
> - svn_boolean_t mutable_rep_truncated, apr_pool_t
> *pool)
>
> + int format,
>
> + svn_boolean_t mutable_rep_truncated,
>
> + apr_pool_t *pool)
>
> {
>
> - /* ###: Make the writing of the sha1 and reuse count dependent on
the
>
> - format number. */
>
> if (rep->txn_id && mutable_rep_truncated)
>
> return "-1";
>
> - else
>
> +
>
> + if (format >= SVN_FS_FS__MIN_REP_SHARING_FORMAT)
>
> return apr_psprintf(pool, "%ld %" APR_OFF_T_FMT " %"
SVN_FILESIZE_T_FMT
>
> " %" SVN_FILESIZE_T_FMT " %s %s %"
APR_INT64_T_FMT,
>
> rep->revision, rep->offset, rep->size,
>
>
>
> If I do a dump/load of a repository, I get a different size of the
> repository with revision 34377 compared with the current version
(34428). If
> I apply this patch on 34428, I get the same repository after a
dump/load.
>
>
>
> Regards,
>
> Marc

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org