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

Re: svn commit: r23282 - in trunk/subversion/svn: . schema

From: Peter Lundblad <plundblad_at_google.com>
Date: 2007-01-30 15:59:10 CET

Hi,

hwright@tigris.org writes:
> Modified: trunk/subversion/svn/cl.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/cl.h?pathrev=23282&r1=23281&r2=23282
> ==============================================================================
> --- trunk/subversion/svn/cl.h (original)
> +++ trunk/subversion/svn/cl.h Mon Jan 29 14:45:40 2007
> @@ -282,6 +282,14 @@
> svn_boolean_t names_only,
> apr_pool_t *pool);
>
> +/* Same as svn_cl__print_prop_hash(), only output xml to SB. If SB is NULL,
> + allocate it first from pool, otherwise append the xml to it. */

Parameter name is outstr, not sb.

> +svn_error_t *
> +svn_cl__print_xml_prop_hash(svn_stringbuf_t **outstr,
> + apr_hash_t *prop_hash,
> + svn_boolean_t names_only,
> + apr_pool_t *pool);
> +
> /* Do the following things that are commonly required before accessing revision
> properties. Ensure that REVISION is specified explicitly and is not
> relative to a working-copy item. Ensure that exactly one target is
>
> Modified: trunk/subversion/svn/main.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/main.c?pathrev=23282&r1=23281&r2=23282
> ==============================================================================
> --- trunk/subversion/svn/main.c (original)
> +++ trunk/subversion/svn/main.c Mon Jan 29 14:45:40 2007
> @@ -613,7 +613,7 @@
> " 2. Lists unversioned remote props on repos revision.\n"
> " TARGET only determines which repository to access.\n"),
> {'v', 'R', 'r', 'q', svn_cl__revprop_opt, SVN_CL__AUTH_OPTIONS,
> - svn_cl__config_dir_opt} },
> + svn_cl__config_dir_opt, svn_cl__xml_opt} },
>
> { "propset", svn_cl__propset, {"pset", "ps"}, N_
> ("Set the value of a property on files, dirs, or revisions.\n"
>
> Modified: trunk/subversion/svn/proplist-cmd.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/proplist-cmd.c?pathrev=23282&r1=23281&r2=23282
> ==============================================================================
> --- trunk/subversion/svn/proplist-cmd.c (original)
> +++ trunk/subversion/svn/proplist-cmd.c Mon Jan 29 14:45:40 2007
> @@ -124,19 +162,36 @@
> /* Check for a peg revision. */
> SVN_ERR(svn_opt_parse_path(&peg_revision, &truepath, target,
> subpool));
> -
> - SVN_ERR(svn_cl__try
> - (svn_client_proplist3(truepath, &peg_revision,
> - &(opt_state->start_revision),
> - opt_state->recursive,
> - proplist_receiver,
> - &pl_baton,
> - ctx, subpool),
> - NULL, opt_state->quiet,
> - SVN_ERR_UNVERSIONED_RESOURCE,
> - SVN_ERR_ENTRY_NOT_FOUND,
> - SVN_NO_ERROR));
> +
> + if (opt_state->xml)
> + SVN_ERR(svn_cl__try
> + (svn_client_proplist3(truepath, &peg_revision,
> + &(opt_state->start_revision),
> + opt_state->recursive,
> + proplist_receiver_xml,
> + &pl_baton,
> + ctx, subpool),
> + NULL, opt_state->quiet,
> + SVN_ERR_UNVERSIONED_RESOURCE,
> + SVN_ERR_ENTRY_NOT_FOUND,
> + SVN_NO_ERROR));
> + else
> + SVN_ERR(svn_cl__try
> + (svn_client_proplist3(truepath, &peg_revision,
> + &(opt_state->start_revision),
> + opt_state->recursive,
> + proplist_receiver,
> + &pl_baton,
> + ctx, subpool),
> + NULL, opt_state->quiet,
> + SVN_ERR_UNVERSIONED_RESOURCE,
> + SVN_ERR_ENTRY_NOT_FOUND,
> + SVN_NO_ERROR));

It looks like the only thing that distinguishes these two calls is the
receiver parameter. I think it is more readable to either put the
opt_state->xml condition inline in the function call or assign a variable
before to avoid this duplication. What do you think?

> Modified: trunk/subversion/svn/props.c
> @@ -106,3 +107,50 @@
>
> return SVN_NO_ERROR;
> }
> +
> +svn_error_t *
> +svn_cl__print_xml_prop_hash(svn_stringbuf_t **outstr,
> + apr_hash_t *prop_hash,
> + svn_boolean_t names_only,
> + apr_pool_t *pool)
> +{
> + apr_hash_index_t *hi;
> +
> + if (*outstr == NULL)
> + *outstr = svn_stringbuf_create("", pool);
> +
> + for (hi = apr_hash_first(pool, prop_hash); hi; hi = apr_hash_next(hi))
> + {
> + const void *key;
> + void *val;
> + const char *pname;
> + svn_string_t *propval;
> +
> + apr_hash_this(hi, &key, NULL, &val);
> + pname = key;
> + propval = val;
> +
> + if (names_only)
> + {
> + svn_xml_make_open_tag(outstr, pool, svn_xml_self_closing, "property",
> + "name", pname, NULL);
> + }
> + else
> + {
> + const char *pname_out;
> +
> + if (svn_prop_needs_translation(pname))
> + SVN_ERR(svn_subst_detranslate_string(&propval, propval,
> + TRUE, pool));
> +
> + SVN_ERR(svn_cmdline_cstring_from_utf8(&pname_out, pname, pool));
> +
> + svn_xml_make_open_tag(outstr, pool, svn_xml_protect_pcdata,
> + "property", "name", pname, NULL);
> + svn_xml_escape_cdata_string(outstr, propval, pool);

If we have a property that doesn't start with "svn:", we can't assume
that it is valid UTF8, so this call will produce non-well-formed XML.
This is also the case if the value contains characters that are not legal in
XML, such as certain control characters.
So, we need to encode values that are not well-formed, in a similar way that
we do in our DAV implementation.

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jan 30 15:59:37 2007

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.