Re: [PATCH] fix for issue #591

Garrett Rooney <rooneg@electricjellyfish.net> writes:

> --- ./subversion/libsvn_client/copy.c
> +++ ./subversion/libsvn_client/copy.c Sun Jan 20 11:55:49 2002
> @@ -534,6 +534,8 @@
> svn_stream_t *fstream;
> apr_file_t *fp;
> svn_revnum_t fetched_rev = 0;
> + apr_hash_t *props;
> + apr_hash_index_t *hi;
>
> /* Open DST_PATH for writing. */
> status = apr_file_open (&fp, dst_path->data, (APR_CREATE | APR_WRITE),
> @@ -549,7 +551,18 @@
> /* Have the RA layer 'push' data at this stream. We pass a
> relative path of "", because we opened SRC_URL, which is
> already the full URL to the file. */
> - SVN_ERR (ra_lib->get_file (sess, "", src_rev, fstream, &fetched_rev));
> + SVN_ERR (ra_lib->get_file (sess, "", src_rev, fstream,
> + &fetched_rev, &props));
> +
> + for (hi = apr_hash_first(pool, props); hi; hi = apr_hash_next(hi))
> + {
> + const char *key;
> + svn_string_t *val;
> +
> + apr_hash_this(hi, (const void **)&key, NULL, (void **)&val);

Subversion style is to put a space between the function and the
opening parenthesis [this affects most of the other function calls in
this patch].

> +
> + SVN_ERR (svn_wc_prop_set (key, val, dst_path->data, pool));
> + }
>
> /* Close the file. */
> status = apr_file_close (fp);
> Index: ./subversion/libsvn_client/repos_diff.c
> ===================================================================
> --- ./subversion/libsvn_client/repos_diff.c
> +++ ./subversion/libsvn_client/repos_diff.c Sat Jan 19 19:29:12 2002
> @@ -254,7 +254,7 @@
> SVN_ERR (b->edit_baton->ra_lib->get_file (b->edit_baton->ra_session,
> b->path->data,
> b->edit_baton->revision,
> - fstream, NULL));
> + fstream, NULL, NULL));
>
> status = apr_file_close (file);
> if (status)
> Index: ./subversion/libsvn_ra_dav/ra_dav.h
> ===================================================================
> --- ./subversion/libsvn_ra_dav/ra_dav.h
> +++ ./subversion/libsvn_ra_dav/ra_dav.h Sat Jan 19 19:27:46 2002
> @@ -85,7 +85,8 @@
> const char *path,
> svn_revnum_t revision,
> svn_stream_t *stream,
> - svn_revnum_t *fetched_rev);
> + svn_revnum_t *fetched_rev,
> + apr_hash_t **props);
>
> svn_error_t * svn_ra_dav__abort_commit(
> void *session_baton,
> Index: ./subversion/libsvn_ra_dav/fetch.c
> ===================================================================
> --- ./subversion/libsvn_ra_dav/fetch.c
> +++ ./subversion/libsvn_ra_dav/fetch.c Sun Jan 20 12:05:35 2002
> @@ -785,8 +785,11 @@
> const char *path,
> svn_revnum_t revision,
> svn_stream_t *stream,
> - svn_revnum_t *fetched_rev)
> + svn_revnum_t *fetched_rev,
> + apr_hash_t **props)
> {
> + svn_ra_dav_resource_t *rsrc;
> + apr_hash_index_t *hi;
> svn_stringbuf_t *url_str;
> const char *final_url;
> svn_ra_session_t *ras = (svn_ra_session_t *) session_baton;
> @@ -828,6 +831,31 @@
> get_file_reader, stream,
> ras->callbacks->get_wc_prop,
> ras->callback_baton, ras->pool) );
> +
> + if (props)
> + {
> + SVN_ERR( svn_ra_dav__get_props_resource(&rsrc, ras->sess, final_url,
> + NULL, NULL /* all props */,
> + ras->pool) );
> +
> + *props = apr_hash_make(ras->pool);
> +
> + for (hi = apr_hash_first(ras->pool, rsrc->propset);
> + hi;
> + hi = apr_hash_next(hi))
> + {
> + const char *key;
> + char *val;
> +
> + apr_hash_this(hi, (const void **)&key, NULL, (void **)&val);

Existing code generally avoids the casts and declares

           const void *key;
           apr_ssize_t klen;
           void *val;
           apr_hash_this (hi, &key, &klen, &val);

> +
> +#define NSLEN (sizeof(SVN_PROP_CUSTOM_PREFIX) - 1)
> + if (strncmp(key, SVN_PROP_CUSTOM_PREFIX, NSLEN) == 0)
> + apr_hash_set(*props, &key[NSLEN], APR_HASH_KEY_STRING,
> + svn_string_create(val, ras->pool));
> +#undef NSLEN

Why the preprocessor abuse? Existing code uses a variable such as
 apr_size_t custom_prefix_len = sizeof (SVN_PROP_CUSTOM_PREFIX) - 1;

However, I worry about the use of strncmp. As far as I can see a user
can create a property called "custom:xxx" which will erroneously
compare equal if you use the above test. Now perhaps that should not
be allowed, but a strcmp, or a klen==custom_prefix_len test, would
avoid the problem.

> + }
> + }
>
> return SVN_NO_ERROR;
> }

-- 
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org