Re: [PATCH] dav_svn_get_repo_name handling of SVNParentPath

On Sat, 14 Jan 2006, David Anderson wrote:

> On Saturday 14 January 2006 15:36, Maxime Petazzoni wrote:
> > [[[
> > Make mod_dav_svn retrieve a repository name even when configured to
> > operate with SVNParentPath.
> >
> > * subversion/mod_dav_svn/mod_dav_svn.c
> > (dav_svn_get_repo_name2): New function, revved from
> > dav_svn_get_repo_name. Takes new root_path and repo_name arguments
> > and returns a dav_error for error propagation. Retrieves the
> > repository name in a SVNParentPath setup using dav_svn_split_uri.

> > (dav_svn_get_repo_name): Deprecate.
...
> > Index: subversion/mod_dav_svn/mod_dav_svn.c
> > ===================================================================
> > --- subversion/mod_dav_svn/mod_dav_svn.c (revision 18075)
> > +++ subversion/mod_dav_svn/mod_dav_svn.c (working copy)
> > @@ -326,6 +326,9 @@
> > return NULL;
> > }
> >
> > +/* Get the name of the repository targetted by the request. Note that
> > + if the configuration uses SVNParentPath, NULL is always returned
> > + (see dav_svn_get_repo_name2). */
> > const char *dav_svn_get_repo_name(request_rec *r)
> > {
> > dav_svn_dir_conf *conf;

Why not refactor this function to call dav_svn_get_repo_name2()?

> > @@ -334,6 +337,47 @@
> > return conf->repo_name;
> > }
> >
> > +dav_error *dav_svn_get_repo_name2(request_rec *r, const char *root_path,
> > + char **repo_name)
> > +{
> > + const char *fs_parent_path;
> > + const char *repos_name;
> > + const char *ignored_path_in_repos;
> > + const char *ignored_cleaned_uri;
> > + const char *ignored_relative;

Many of these parameters can be scoped to inside the
"if (fs_parent_path != NULL)" block below.

> > + int ignored_had_slash;
> > + dav_error *derr;
> > + dav_svn_dir_conf *conf;
> > +
> > + /* Handle the SVNParentPath case. When this directive is used, we
> > + can't retrieve the repository name directly from the module's
> > + configuration structure : we have to fetch it from the URI. */
> > + fs_parent_path = dav_svn_get_fs_parent_path(r);
> > +
> > + if (fs_parent_path != NULL)
> > + {
> > + /* Split the svn URI to get the name of the repository below
> > + the parent path. */
> > + derr = dav_svn_split_uri(r, r->uri, root_path,
> > + &ignored_cleaned_uri, &ignored_had_slash,
> > + &repos_name,
> > + &ignored_relative, &ignored_path_in_repos);
> > +
> > + if (derr)
> > + return derr;
> > +
> > + *repo_name = repos_name;
> > + return NULL;
> > + }
> > +
> > + /* Otherwise, return the repository name from the SVNPath
> > + configuration directive. */
> > + conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);
> > + *repo_name = conf->repo_name;
> > +
> > + return NULL;
> > +}
> > +
> > const char *dav_svn_get_xslt_uri(request_rec *r)
> > {
> > dav_svn_dir_conf *conf;
> > Index: subversion/mod_dav_svn/dav_svn.h
> > ===================================================================
> > --- subversion/mod_dav_svn/dav_svn.h (revision 18075)
> > +++ subversion/mod_dav_svn/dav_svn.h (working copy)
> > @@ -313,6 +313,8 @@
> >
> > /* Return a descriptive name for the repository */
> > const char *dav_svn_get_repo_name(request_rec *r);
> > +dav_error *dav_svn_get_repo_name2(request_rec *r, const char *root_path,
> > + char **repo_name);
...

Some of the function decls in this header file are marked up for
Doxygen. If we're running Doxygen on this header file, this func
could also use some markup. Also parameter and @since doucmentation.

-- 
Daniel Rall