> -----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