Author: kotkov
Date: Fri Sep 8 22:24:39 2017
New Revision: 1807833
URL: http://svn.apache.org/viewvc?rev=1807833&view=rev
Log:
Avoid a function with a potentially unexpected side effect by introducing
an output parameter instead of changing the value in place.
* subversion/svnrdump/svnrdump.h
(svn_rdump__normalize_prop): Add output parameter and update the docstring.
(svn_rdump__normalize_props): Update the docstring.
* subversion/svnrdump/util.c
(svn_rdump__normalize_prop): Populate the output parameter. Ensure
that the resulting value is always copied (even if unchanged) to the
result pool; that should also allow to sometimes get rid of the double
copying on the calling site.
(svn_rdump__normalize_props): Update call to svn_rdump__normalize_prop().
* subversion/svnrdump/load_editor.c
(set_revision_property, set_node_property): Update these calling sites of
svn_rdump__normalize_prop(). Don't make unnecessary copies, as
now the values are always copied to the result pool during the call
to svn_rdump__normalize_prop().
Modified:
subversion/trunk/subversion/svnrdump/load_editor.c
subversion/trunk/subversion/svnrdump/svnrdump.h
subversion/trunk/subversion/svnrdump/util.c
Modified: subversion/trunk/subversion/svnrdump/load_editor.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/load_editor.c?rev=1807833&r1=1807832&r2=1807833&view=diff
==============================================================================
--- subversion/trunk/subversion/svnrdump/load_editor.c (original)
+++ subversion/trunk/subversion/svnrdump/load_editor.c Fri Sep 8 22:24:39 2017
@@ -713,7 +713,7 @@ set_revision_property(void *baton,
{
struct revision_baton *rb = baton;
- SVN_ERR(svn_rdump__normalize_prop(name, &value, rb->pool));
+ SVN_ERR(svn_rdump__normalize_prop(&value, name, value, rb->pool));
SVN_ERR(svn_repos__validate_prop(name, value, rb->pool));
@@ -721,8 +721,7 @@ set_revision_property(void *baton,
{
if (! svn_hash_gets(rb->pb->skip_revprops, name))
svn_hash_sets(rb->revprop_table,
- apr_pstrdup(rb->pool, name),
- svn_string_dup(value, rb->pool));
+ apr_pstrdup(rb->pool, name), value);
}
else if (rb->rev_offset == -1
&& ! svn_hash_gets(rb->pb->skip_revprops, name))
@@ -737,9 +736,9 @@ set_revision_property(void *baton,
/* Remember any datestamp/ author that passes through (see comment
in close_revision). */
if (!strcmp(name, SVN_PROP_REVISION_DATE))
- rb->datestamp = svn_string_dup(value, rb->pool);
+ rb->datestamp = value;
if (!strcmp(name, SVN_PROP_REVISION_AUTHOR))
- rb->author = svn_string_dup(value, rb->pool);
+ rb->author = value;
return SVN_NO_ERROR;
}
@@ -776,13 +775,13 @@ set_node_property(void *baton,
value = new_value;
}
- SVN_ERR(svn_rdump__normalize_prop(name, &value, pool));
+ SVN_ERR(svn_rdump__normalize_prop(&value, name, value, pool));
SVN_ERR(svn_repos__validate_prop(name, value, pool));
prop = apr_palloc(nb->rb->pool, sizeof (*prop));
prop->name = apr_pstrdup(pool, name);
- prop->value = svn_string_dup(value, pool);
+ prop->value = value;
svn_hash_sets(nb->prop_changes, prop->name, prop);
return SVN_NO_ERROR;
Modified: subversion/trunk/subversion/svnrdump/svnrdump.h
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/svnrdump.h?rev=1807833&r1=1807832&r2=1807833&view=diff
==============================================================================
--- subversion/trunk/subversion/svnrdump/svnrdump.h (original)
+++ subversion/trunk/subversion/svnrdump/svnrdump.h Fri Sep 8 22:24:39 2017
@@ -100,9 +100,6 @@ svn_rdump__load_dumpstream(svn_stream_t
* currently all svn:* props) so that they contain only LF (\n) line endings.
*
* Put the normalized props into NORMAL_PROPS, allocated in RESULT_POOL.
- *
- * Note: this function does not do a deep copy; it is expected that PROPS has
- * a longer lifetime than NORMAL_PROPS.
*/
svn_error_t *
svn_rdump__normalize_props(apr_hash_t **normal_props,
@@ -116,11 +113,14 @@ svn_rdump__normalize_props(apr_hash_t **
* "\r\n" sequences are replaced with "\n"
*
* NAME is used to check that VALUE should be normalized, and if this is the
- * case, VALUE is then normalized, allocated from RESULT_POOL
+ * case, *RESULT_P is then set to the normalized property value. If no
+ * normalization is required, VALUE will be copied to RESULT_POOL
+ * unchanged.
*/
svn_error_t *
-svn_rdump__normalize_prop(const char *name,
- const svn_string_t **value,
+svn_rdump__normalize_prop(const svn_string_t **result_p,
+ const char *name,
+ const svn_string_t *value,
apr_pool_t *result_pool);
#ifdef __cplusplus
Modified: subversion/trunk/subversion/svnrdump/util.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/util.c?rev=1807833&r1=1807832&r2=1807833&view=diff
==============================================================================
--- subversion/trunk/subversion/svnrdump/util.c (original)
+++ subversion/trunk/subversion/svnrdump/util.c Fri Sep 8 22:24:39 2017
@@ -31,21 +31,27 @@
svn_error_t *
-svn_rdump__normalize_prop(const char *name,
- const svn_string_t **value,
+svn_rdump__normalize_prop(const svn_string_t **result_p,
+ const char *name,
+ const svn_string_t *value,
apr_pool_t *result_pool)
{
- if (svn_prop_needs_translation(name) && *value)
+ if (svn_prop_needs_translation(name) && value)
{
const char *cstring;
- SVN_ERR(svn_subst_translate_cstring2((*value)->data, &cstring,
+ SVN_ERR(svn_subst_translate_cstring2(value->data, &cstring,
"\n", TRUE,
NULL, FALSE,
result_pool));
- *value = svn_string_create(cstring, result_pool);
+ *result_p = svn_string_create(cstring, result_pool);
}
+ else
+ {
+ *result_p = svn_string_dup(value, result_pool);
+ }
+
return SVN_NO_ERROR;
}
@@ -64,8 +70,7 @@ svn_rdump__normalize_props(apr_hash_t **
const char *key = apr_hash_this_key(hi);
const svn_string_t *value = apr_hash_this_val(hi);
- SVN_ERR(svn_rdump__normalize_prop(key, &value,
- result_pool));
+ SVN_ERR(svn_rdump__normalize_prop(&value, key, value, result_pool));
svn_hash_sets(*normal_props, key, value);
}
