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

RE: svn commit: r1711582 - in /subversion/trunk/subversion: libsvn_fs_fs/tree.c tests/libsvn_fs/fs-test.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Sat, 31 Oct 2015 15:01:28 +0100

> -----Original Message-----
> From: ivan_at_apache.org [mailto:ivan_at_apache.org]
> Sent: zaterdag 31 oktober 2015 10:32
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1711582 - in /subversion/trunk/subversion:
> libsvn_fs_fs/tree.c tests/libsvn_fs/fs-test.c
>
> Author: ivan
> Date: Sat Oct 31 09:31:46 2015
> New Revision: 1711582
>
> URL: http://svn.apache.org/viewvc?rev=1711582&view=rev
> Log:
> Avoid double DAG lookup in FSFS implementation of
> svn_fs_contents_changed()
> and svn_fs_contents_different().
>
> It also slightly changes error message when these invoked functions invoked
> for non-existent path: before this change error message was "'/non-existent'
> is not a file" now it will be "File not found: revision 1, path
> '/non-existent'"
>
> * subversion/libsvn_fs_fs/tree.c
> (fs_contents_changed): Use svn_fs_fs__dag_node_kind() to get node kind
> of
> already obtained dag_node_t instead of calling to
> svn_fs_fs__check_path().
>
> * subversion/tests/libsvn_fs/fs-test.c
> (compare_contents): Extend test to test behavior of
> svn_fs_contents_changed() and svn_fs_contents_different() with
> directories
> and non-existent paths.
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_fs/tree.c
> subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tr
> ee.c?rev=1711582&r1=1711581&r2=1711582&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Sat Oct 31 09:31:46
> 2015
> @@ -3235,23 +3235,18 @@ fs_contents_changed(svn_boolean_t *chang
> (SVN_ERR_FS_GENERAL, NULL,
> _("Cannot compare file contents between two different filesystems"));
>
> - /* Check that both paths are files. */
> - {
> - svn_node_kind_t kind;
> -
> - SVN_ERR(svn_fs_fs__check_path(&kind, root1, path1, pool));
> - if (kind != svn_node_file)
> - return svn_error_createf
> - (SVN_ERR_FS_GENERAL, NULL, _("'%s' is not a file"), path1);
> -
> - SVN_ERR(svn_fs_fs__check_path(&kind, root2, path2, pool));
> - if (kind != svn_node_file)
> - return svn_error_createf
> - (SVN_ERR_FS_GENERAL, NULL, _("'%s' is not a file"), path2);
> - }
> -
> SVN_ERR(get_dag(&node1, root1, path1, pool));
> + /* Make sure that path is file. */
> + if (svn_fs_fs__dag_node_kind(node1) != svn_node_file)
> + return svn_error_createf
> + (SVN_ERR_FS_GENERAL, NULL, _("'%s' is not a file"), path1);
> +
> SVN_ERR(get_dag(&node2, root2, path2, pool));
> + /* Make sure that path is file. */
> + if (svn_fs_fs__dag_node_kind(node2) != svn_node_file)
> + return svn_error_createf
> + (SVN_ERR_FS_GENERAL, NULL, _("'%s' is not a file"), path2);
> +

I don't think you want to backport this, but returning SVN_ERR_FS_NOT_FILE would make more sense here, than returning SVN_ERR_FS_GENERAL.

And I would like to see the regression test checking that for all filesystem layers for 1.10... but that is a bigger change.

        Bert
Received on 2015-10-31 15:01:37 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.