[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r38533 - trunk/subversion/mod_dav_svn

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: Tue, 04 Aug 2009 16:44:21 +0530

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

C. Michael Pilato wrote:
> Author: cmpilato
> Date: Mon Aug 3 11:54:59 2009
> New Revision: 38533
>
> Log:
> Make some bugfixes and optimizations to the DAV mirroring code.
>
> * subversion/mod_dav_svn/mirror.c
> (locate_ctx_t): Remove unnecessary 'uri' member.
> (dav_svn__location_in_filter, dav_svn__location_body_filter): Add
> some comments. Avoid filtering if doing so would be a no-op
> (replacing a string with itself). Switch to case-sensitive string
> replacements.
> (dav_svn__location_header_filter): Add comments, and reorganize the
> code to match the patterns used in the other filters.
>
> Modified:
> trunk/subversion/mod_dav_svn/mirror.c
>
> Modified: trunk/subversion/mod_dav_svn/mirror.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/mod_dav_svn/mirror.c?pathrev=38533&r1=38532&r2=38533
> ==============================================================================
> --- trunk/subversion/mod_dav_svn/mirror.c Mon Aug 3 11:20:03 2009 (r38532)
> +++ trunk/subversion/mod_dav_svn/mirror.c Mon Aug 3 11:54:59 2009 (r38533)
> @@ -109,7 +109,6 @@ typedef struct locate_ctx_t
> {
> const apr_strmatch_pattern *pattern;
> apr_size_t pattern_len;
> - apr_uri_t uri;
> const char *localpath;
> apr_size_t localpath_len;
> const char *remotepath;
> @@ -126,15 +125,26 @@ apr_status_t dav_svn__location_in_filter
> locate_ctx_t *ctx = f->ctx;
> apr_status_t rv;
> apr_bucket *bkt;
> - const char *master_uri;
> + const char *master_uri, *root_dir;
> + apr_uri_t uri;
>
> + /* Don't filter if we're in a subrequest or we aren't setup to
> + proxy anything. */
> master_uri = dav_svn__get_master_uri(r);
> -
> if (r->main || !master_uri) {
> ap_remove_input_filter(f);
> return ap_get_brigade(f->next, bb, mode, block, readbytes);
> }
>
> + /* And don't filter if our search-n-replace would be a noop anyway
> + (that is, if our root path matches that of the master server). */
> + apr_uri_parse(r->pool, master_uri, &uri);
> + root_dir = dav_svn__get_root_dir(r);
> + if (strcmp(master_uri, root_dir) == 0) {

This should be strcmp(uri.path, root_dir).

> + ap_remove_input_filter(f);
> + return ap_get_brigade(f->next, bb, mode, block, readbytes);
> + }
> +
> /* ### FIXME: While we want to fix up any locations in proxied XML
> ### requests, we do *not* want to be futzing with versioned (or
> ### to-be-versioned) data, such as the file contents present in
> @@ -143,13 +153,11 @@ apr_status_t dav_svn__location_in_filter
>
> if (!f->ctx) {
> ctx = f->ctx = apr_pcalloc(r->pool, sizeof(*ctx));
> -
> - apr_uri_parse(r->pool, master_uri, &ctx->uri);
> - ctx->remotepath = ctx->uri.path;
> + ctx->remotepath = uri.path;
> ctx->remotepath_len = strlen(ctx->remotepath);
> - ctx->localpath = dav_svn__get_root_dir(r);
> + ctx->localpath = root_dir;
> ctx->localpath_len = strlen(ctx->localpath);
> - ctx->pattern = apr_strmatch_precompile(r->pool, ctx->localpath, 0);
> + ctx->pattern = apr_strmatch_precompile(r->pool, ctx->localpath, 1);
> ctx->pattern_len = ctx->localpath_len;
> }
>
> @@ -196,29 +204,31 @@ apr_status_t dav_svn__location_header_fi
> {
> request_rec *r = f->r;
> const char *master_uri;
> + const char *location, *start_foo = NULL;
>
> + /* Don't filter if we're in a subrequest or we aren't setup to
> + proxy anything. */
> master_uri = dav_svn__get_master_uri(r);
> -
> if (!r->main && master_uri) {
> - const char *location, *start_foo = NULL;
> + ap_remove_output_filter(f);
> + return ap_pass_brigade(f->next, bb);
> + }
>
> - location = apr_table_get(r->headers_out, "Location");
> - if (location) {
> - start_foo = ap_strstr_c(location, master_uri);
> - }
> - if (start_foo) {
> - const char *new_uri;
> - start_foo += strlen(master_uri);
> - new_uri = ap_construct_url(r->pool,
> - apr_pstrcat(r->pool,
> - dav_svn__get_root_dir(r),
> - start_foo, NULL),
> - r);
> - apr_table_set(r->headers_out, "Location", new_uri);
> - }
> + location = apr_table_get(r->headers_out, "Location");
> + if (location) {
> + start_foo = ap_strstr_c(location, master_uri);
> + }
> + if (start_foo) {
> + const char *new_uri;
> + start_foo += strlen(master_uri);
> + new_uri = ap_construct_url(r->pool,
> + apr_pstrcat(r->pool,
> + dav_svn__get_root_dir(r),
> + start_foo, NULL),
> + r);
> + apr_table_set(r->headers_out, "Location", new_uri);
> }
> - ap_remove_output_filter(f);
> - return ap_pass_brigade(f->next, bb);
> + return APR_SUCCESS;
> }
>
> apr_status_t dav_svn__location_body_filter(ap_filter_t *f,
> @@ -227,15 +237,26 @@ apr_status_t dav_svn__location_body_filt
> request_rec *r = f->r;
> locate_ctx_t *ctx = f->ctx;
> apr_bucket *bkt;
> - const char *master_uri;
> + const char *master_uri, *root_dir;
> + apr_uri_t uri;
>
> + /* Don't filter if we're in a subrequest or we aren't setup to
> + proxy anything. */
> master_uri = dav_svn__get_master_uri(r);
> -
> if (r->main || !master_uri) {
> ap_remove_output_filter(f);
> return ap_pass_brigade(f->next, bb);
> }
>
> + /* And don't filter if our search-n-replace would be a noop anyway
> + (that is, if our root path matches that of the master server). */
> + apr_uri_parse(r->pool, master_uri, &uri);
> + root_dir = dav_svn__get_root_dir(r);
> + if (strcmp(master_uri, root_dir) == 0) {

This should be strcmp(uri.path, root_dir).

Fixed both in r38551.

With regards
Kamesh Jayachandran
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iD8DBQFKeBgM3WHvyO0YTCwRAtcYAJ4x4kSLLXHzvP4lARuc2jbM7y4DIgCfWreD
OCbRHJ9F0SClvg3VQpFoEz0=
=Xx3b
-----END PGP SIGNATURE-----

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2379963
Received on 2009-08-04 13:15:00 CEST

This is an archived mail posted to the Subversion Dev mailing list.