Graham Leggett <minf...@sharp.fm> writes:
> + if (arg2) {
> +#if AP_MODULE_MAGIC_AT_LEAST(20111025,3)
> + const char *expr_err = NULL;
> +
> + conf->fs_path->expr = ap_expr_parse_cmd(cmd, arg2,
> AP_EXPR_FLAG_STRING_RESULT,
> + &expr_err, NULL);
> + if (expr_err) {
> + apr_pstrcat(cmd->temp_pool,
> + "Cannot parse expression in SVNPath: ",
> + expr_err, NULL);
expr_err contains the error message if parsing fails but it appears to
be discared.
> + }
You have an unmatched } inside the #if but not in the #else, one of them
is wrong.
> +#else
> + return "Expressions require httpd v2.4.0 or higher"
> +#endif
> + }
> +
> return NULL;
> }
>
>
> static const char *
> -SVNParentPath_cmd(cmd_parms *cmd, void *config, const char *arg1)
> +SVNParentPath_cmd(cmd_parms *cmd, void *config, const char *arg1, const char
> *arg2)
> {
> dir_conf_t *conf = config;
>
> if (conf->fs_path != NULL)
> return "SVNParentPath cannot be defined at same time as SVNPath.";
>
> - conf->fs_parent_path = svn_dirent_internal_style(arg1, cmd->pool);
> + conf->fs_parent_path = apr_pcalloc(cmd->pool, sizeof(path_expr_t));
> + conf->fs_parent_path->base = svn_dirent_internal_style(arg1, cmd->pool);
>
> + if (arg2) {
> +#if AP_MODULE_MAGIC_AT_LEAST(20111025,3)
> + const char *expr_err = NULL;
> +
> + conf->fs_path->expr = ap_expr_parse_cmd(cmd, arg2,
> AP_EXPR_FLAG_STRING_RESULT,
> + &expr_err, NULL);
> + if (expr_err) {
> + apr_pstrcat(cmd->temp_pool,
> + "Cannot parse expression in SVNParentPath: ",
> + expr_err, NULL);
> + }
> +#else
> + return "Expressions require httpd v2.4.0 or higher"
> +#endif
> + }
Same problems as above in this repeated code. It's relatively small but
perhaps factor into a separate function?
> +
> return NULL;
> }
>
> @@ -668,7 +713,61 @@
> dir_conf_t *conf;
>
> conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);
> - return conf->fs_path;
> +
> + if (conf->fs_path)
> + {
> +
> + #if AP_MODULE_MAGIC_AT_LEAST(20111025,3)
> + if (conf->fs_path->expr)
> + {
> + svn_error_t *serr;
> + svn_boolean_t under_root;
> + const char *err = NULL, *suffix;
> +
> + suffix = ap_expr_str_exec(r, conf->fs_path->expr, &err);
> + if (!err)
> + {
> + serr = svn_dirent_is_under_root(&under_root,
> + &suffix, conf->fs_path->base, suffix, r->pool);
> + if (!serr && under_root)
> + {
> + return svn_dirent_join(conf->fs_path->base,
> + svn_dirent_internal_style(suffix, r->pool),
> r->pool);
> + }
> + else if (serr)
> + {
> + ap_log_rerror(APLOG_MARK, APLOG_ERR, serr->apr_err, r,
> + "mod_dav_svn: SVNParentPath: '%s' not under '%s':
> '%s'",
> + suffix, conf->fs_path->base,
> + serr->message ? serr->message : "(no more info)");
> + return NULL;
> + }
> + else
> + {
> + ap_log_rerror(
> + APLOG_MARK, APLOG_ERR, 0, r,
> + "mod_dav_svn: SVNParentPath: '%s' not under '%s'",
> + suffix, conf->fs_path->base);
> + return NULL;
> + }
> + }
> + else
> + {
> + ap_log_rerror(
> + APLOG_MARK, APLOG_ERR, 0, r, "mod_dav_svn:
> SVNParentPath: can't "
> + "evaluate expression: %s", err);
> + return NULL;
> + }
> +
> + }
> + #endif
This bit of code is also repeated and is probably big enough to be
factored into a separate function.
> +
> + return conf->fs_path->base;
> + }
> + else
> + {
> + return NULL;
> + }
> }
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
