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

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Branko Čibej
On 27.11.2018 19:10, [hidden email] wrote:
> Author: kotkov
> Date: Tue Nov 27 18:10:01 2018
> New Revision: 1847572
>
> URL: http://svn.apache.org/viewvc?rev=1847572&view=rev
> Log:
> fsfs: Fix an issue with the DAG open_path() that was causing unexpected
> SVN_ERR_FS_NOT_DIRECTORY errors when attempting to open a path with
> `open_path_node_only | open_path_allow_null` flags.

[...]


> @@ -1016,12 +1035,26 @@ open_path(parent_path_t **parent_path_p,
>            SVN_ERR(dag_node_cache_get(&here, root, directory, pool));
>  
>            /* Did the shortcut work? */
> -          if (here)
> +          if (here && svn_fs_fs__dag_node_kind(here) == svn_node_dir)
>              {
>                apr_size_t dirname_len = strlen(directory);
>                path_so_far->len = dirname_len;
>                rest = path + dirname_len + 1;
>              }
> +          else if (here)
> +            {
> +              /* The parent node is not a directory.  We are looking for some
> +                 sub-path, so that sub-path will not exist.  That will be o.k.
> +                 if we are just here to check for the path's existence, but
> +                 should result in an error otherwise. */
> +              if (flags & open_path_allow_null)
> +                {
> +                  *parent_path_p = NULL;
> +                  return SVN_NO_ERROR;
> +                }
> +              else
> +                return err_not_directory(root, directory, pool);
> +            }
>          }
>      }
>  


Would be nice to wrap this return value in svn_error_trace(), as that
makes it easier to find where the error came from.


> @@ -1144,8 +1177,6 @@ open_path(parent_path_t **parent_path_p,
>        /* The path isn't finished yet; we'd better be in a directory.  */
>        if (svn_fs_fs__dag_node_kind(child) != svn_node_dir)
>          {
> -          const char *msg;
> -
>            /* Since this is not a directory and we are looking for some
>               sub-path, that sub-path will not exist.  That will be o.k.,
>               if we are just here to check for the path's existence. */
> @@ -1156,14 +1187,7 @@ open_path(parent_path_t **parent_path_p,
>              }
>  
>            /* It's really a problem ... */
> -          msg = root->is_txn_root
> -              ? apr_psprintf(iterpool,
> -                             _("Failure opening '%s' in transaction '%s'"),
> -                             path, root->txn)
> -              : apr_psprintf(iterpool,
> -                             _("Failure opening '%s' in revision %ld"),
> -                             path, root->rev);
> -          SVN_ERR_W(SVN_FS__ERR_NOT_DIRECTORY(fs, path_so_far->data), msg);
> +          return err_not_directory(root, path_so_far->data, iterpool);
>          }

And here.


-- Brane

Reply | Threaded
Open this post in threaded view
|

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

Evgeny Kotkov
Branko Čibej <[hidden email]> writes:

> > +              else
> > +                return err_not_directory(root, directory, pool);
> > +            }
> >          }
> >      }
> >
>
> Would be nice to wrap this return value in svn_error_trace(), as that
> makes it easier to find where the error came from.

Makes sense, committed in https://svn.apache.org/r1847596


Thanks,
Evgeny Kotkov