On Thu, Mar 13, 2003 at 03:30:17PM -0600, cmpilato@tigris.org wrote:
>...
> * subversion/libsvn_ra_dav/commit.c
> (resource_baton_t): Make prop_changes an apr_hash_t.
> (record_prop_change): Update use of prop_changes hash. Also, don't
> do XML-escaping here (we'll do it later).
> (do_setprop): Change prototype (no longer is a callback function for
> apr_table_do). Do XML-escaping here now. Finally, use #defines for this
> stuff now.
In record_prop_change, you make a copy. And then in do_setprop, you make
*another* copy while you escape it. Why defer escaping it? It should be
possible to escape it properly (xml vs binary) right up front.
>...
> +++ trunk/subversion/libsvn_ra_dav/commit.c Thu Mar 13 15:29:51 2003
> @@ -110,7 +110,7 @@
> {
> commit_ctx_t *cc;
> resource_t *rsrc;
> - apr_table_t *prop_changes; /* name/values pairs of changed (or new) properties. */
> + apr_hash_t *prop_changes; /* name/values pairs of new/changed properties. */
> apr_array_header_t *prop_deletes; /* names of properties to delete. */
Now that we've switched over to a hash table, we can follow through on my
original intent. I had defined a "singleton" value when I first wrote this
file. You can record deletes by simply using a pointer to that singleton.
When you iterate the hash table, then you see that val==singleton, and take
the deletion path.
That simplifies the record keeping quite a bit.
>...
> +static int do_setprop(ne_buffer *body,
> + const char *name,
> + svn_string_t *value,
> + apr_pool_t *pool)
> +{
> + /* Map property names to namespaces */
> +#define NSLEN (sizeof(SVN_PROP_PREFIX) - 1)
> + if (strncmp(name, SVN_PROP_PREFIX, NSLEN) == 0)
> + {
> + svn_stringbuf_t *xml_esc = NULL;
> + svn_xml_escape_cdata_string(&xml_esc, value, pool);
> + ne_buffer_concat(body, "<S:", name + NSLEN, ">", xml_esc->data,
> + "</S:", name + NSLEN, ">", NULL);
> + }
> +#undef NSLEN
> + else
> + {
> + svn_stringbuf_t *xml_esc = NULL;
> + svn_xml_escape_cdata_string(&xml_esc, value, pool);
> + ne_buffer_concat(body, "<C:", name, ">", xml_esc->data,
> + "</C:", name, ">", NULL);
> + }
Besides the comment about double-copying into the pools, note that there is
a third copy into the Neon buffer :-)
Anyways, you can factor out the escaped string in the above code.
>...
> +++ trunk/subversion/libsvn_ra_dav/fetch.c Thu Mar 13 15:29:51 2003
>...
> +/* Populate the members of ne_propname structure *PROP for the
> + Subversion property NAME. */
> +static void
> +make_ne_propname (ne_propname *prop,
> + const char *name)
> +{
> + if (strncmp(name, SVN_PROP_PREFIX, (sizeof (SVN_PROP_PREFIX) - 1)) == 0)
Spaces between functions/parens (!)
>...
> {
> + const svn_string_t *propval = value;
> const char *val = NULL;
> svn_ra_session_t *ras = session_baton;
> svn_ra_dav_resource_t *baseline;
> - svn_boolean_t is_svn_prop;
> int rv;
> - static ne_propname propname_struct = {0, 0};
> +
> ne_proppatch_operation po[2] = { { 0 } };
> - ne_propname wanted_props[] =
> + ne_propname prop, wanted_props[] =
> {
This file uses one declaration per line. That style helps readability,
especially, when you have initializers.
>...
> - if (value)
> - {
> - svn_stringbuf_t *valstr = NULL;
> - svn_xml_escape_cdata_cstring(&valstr, value->data, pool);
> - val = valstr->data;
> - }
> + make_ne_propname(&prop, name);
> +
> + if (propval)
> + {
> + svn_stringbuf_t *propesc = NULL;
> + svn_xml_escape_cdata_cstring(&propesc, propval->data, pool);
> + val = propesc->data;
> + }
What's up with the indentation here? Looks like things shifted to the right
by a space.
>...
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Mar 14 05:09:30 2003