bre...@apache.org writes:
> Author: breser
> Date: Thu Nov 14 03:08:33 2013
> New Revision: 1541793
>
> URL: http://svn.apache.org/r1541793
> Log:
> On 1.8.x-r1541790 branch: Merge r1541790 from trunk.
>
> Modified:
> subversion/branches/1.8.x-r1541790/ (props changed)
> subversion/branches/1.8.x-r1541790/subversion/mod_dav_svn/mod_dav_svn.c
>
> Propchange: subversion/branches/1.8.x-r1541790/
> ------------------------------------------------------------------------------
> Merged /subversion/trunk:r1541790
>
> Modified:
> subversion/branches/1.8.x-r1541790/subversion/mod_dav_svn/mod_dav_svn.c
> URL:
> http://svn.apache.org/viewvc/subversion/branches/1.8.x-r1541790/subversion/mod_dav_svn/mod_dav_svn.c?rev=1541793&r1=1541792&r2=1541793&view=diff
> ==============================================================================
> --- subversion/branches/1.8.x-r1541790/subversion/mod_dav_svn/mod_dav_svn.c
> (original)
> +++ subversion/branches/1.8.x-r1541790/subversion/mod_dav_svn/mod_dav_svn.c
> Thu Nov 14 03:08:33 2013
> @@ -1101,7 +1101,8 @@ static int dav_svn__handler(request_rec
> * that %f in logging formats will show as "svn:/path/to/repo/path/in/repo".
> */
> static int dav_svn__translate_name(request_rec *r)
> {
> - const char *fs_path, *repos_basename, *repos_path, *slash;
> + const char *fs_path, *repos_path, *slash;
> + const char *repos_basename = NULL;
> const char *ignore_cleaned_uri, *ignore_relative_path;
> int ignore_had_slash;
> dav_error *err;
> @@ -1112,20 +1113,39 @@ static int dav_svn__translate_name(reque
> return DECLINED;
>
> /* Retrieve path to repo and within repo for the request */
> - if ((err = dav_svn_split_uri(r, r->uri, conf->root_dir,
> &ignore_cleaned_uri,
> - &ignore_had_slash, &repos_basename,
> - &ignore_relative_path, &repos_path)))
> - {
> - dav_svn__log_err(r, err, APLOG_ERR);
> - return HTTP_INTERNAL_SERVER_ERROR;
> - }
> + err = dav_svn_split_uri(r, r->uri, conf->root_dir, &ignore_cleaned_uri,
> + &ignore_had_slash, &repos_basename,
> + &ignore_relative_path, &repos_path);
> +
> if (conf->fs_parent_path)
> {
> + if (err)
> + {
> + if (!repos_basename)
That looks odd. When dav_svn_split_uri returns an error I suppose it
could have set repos_basename to a valid value but that is not how we
normally handle errors. Do we need to make the assumption that
repos_basename could be valid even when an error is returned? We now
need to document and ensure that dav_svn_split_uri never returns an
invalid value here even when it returns an error.
I think it would be better to remove this test, and the initialization
of repos_basename to NULL, and then always set repos_basename at this
point.
> + {
> + /* detect that there is no repos_basename. We can't error out
> + * here due to this because it would mean that
> SVNListParentPath
> + * wouldn't work. So set things up to just use the parent path
> + * as our bogus path. */
> + repos_basename = "";
> + repos_path = NULL;
> + }
> + else
> + {
> + dav_svn__log_err(r, err, APLOG_ERR);
> + return err->status;
> + }
> + }
> fs_path = svn_dirent_join(conf->fs_parent_path, repos_basename,
> r->pool);
> }
> else
> {
> + if (err)
> + {
> + dav_svn__log_err(r, err, APLOG_ERR);
> + return err->status;
> + }
> fs_path = conf->fs_path;
> }
>
>
>
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
