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

Re: svn commit: rev 763 - trunk/subversion/libsvn_wc trunk/subversion/libsvn_ra_local trunk/subversion/mod_dav_svn trunk/subversion/tests/clients/cmdline

From: Greg Stein <gstein_at_lyra.org>
Date: 2002-01-09 02:06:56 CET

On Tue, Jan 08, 2002 at 02:18:23PM -0600, sussman@tigris.org wrote:
>...
> * mod_dav_svn/update.c (close_helper): if the author or date are NULL,
> then output "" as the property value (xml cdata).

The property just shouldn't be sent. If the values are NULL, then the
properties don't exist. "" is an empty author.

> On the client
> end, these should still come across as NULL by expat.

Expat makes no such guarantee. I'm not sure what you're seeing that says a
value would be NULL. I would think that expat would pass empty strings for
empty cdata.

Can you explain this comment a bit more?

> * libsvn_wc/get_editor.c (change_dir_prop, change_file_prop): if the
> update editor receives an 'svn:wc:entry:" property with NULL value,
> then set the entry attribute's value to "".

I would recommend just omitting the property rather than substituting an
empty string. The two seem to be very different.

>...
> +++ NEW/trunk/subversion/libsvn_wc/get_editor.c Tue Jan 8 14:18:21 2002
>...
> - /* push the property into the hash */
> - apr_hash_set (att_hash, local_name->data, local_name->len, local_value);
> + /* push the property into the att hash. */
> + if (local_value)
> + apr_hash_set (att_hash, local_name->data,
> + local_name->len, local_value);
> + else
> + apr_hash_set (att_hash, local_name->data,
> + local_name->len, "");

Per my comment above, I'd think that reverting this little bit would get the
property removed when the value is NULL.

Alternatively, you might want to say, "woah. something could already be
there and we don't want to remove it." In that case, just don't do the set.
However, I would think you could end up with stale data in that case, so
removing the property when you get a NULL is probably the best behavior.

>...
> @@ -1498,7 +1503,9 @@
> SVN_WC__LOG_ATTR_NAME,
> fb->name,
> prop->name->data,
> - prop->value,
> + prop->value ?
> + prop->value :
> + svn_stringbuf_create ("", fb->pool),
> NULL);

Not sure what is right here...

>...
> +++ NEW/trunk/subversion/libsvn_ra_local/update_editor.c Tue Jan 8 14:18:22 2002
> @@ -96,17 +96,24 @@
> &last_author,
> root, path, subpool));
>
> + /* A root/path will always have a "created rev" field. */
> revision_str = apr_psprintf (subpool, "%ld", committed_rev);
> name = svn_stringbuf_create (SVN_PROP_ENTRY_COMMITTED_REV, subpool);
> value = svn_stringbuf_create (revision_str, subpool);
> SVN_ERR ((*pset_func) (real_baton, name, value));
> -
> - name = svn_stringbuf_create (SVN_PROP_ENTRY_COMMITTED_DATE, subpool);
> - value = svn_stringbuf_create_from_string (committed_date, subpool);
> +
> + if (committed_date)
> + {
> + name = svn_stringbuf_create (SVN_PROP_ENTRY_COMMITTED_DATE, subpool);
> + value = svn_stringbuf_create_from_string (committed_date, subpool);
> + }
> SVN_ERR ((*pset_func) (real_baton, name, value));

If committed_date == NULL, then you'll end up setting the REV again. The
pset_func call ought to go inside the "if" block.

> - name = svn_stringbuf_create (SVN_PROP_ENTRY_LAST_AUTHOR, subpool);
> - value = svn_stringbuf_create_from_string (last_author, subpool);
> + if (last_author)
> + {
> + name = svn_stringbuf_create (SVN_PROP_ENTRY_LAST_AUTHOR, subpool);
> + value = svn_stringbuf_create_from_string (last_author, subpool);
> + }
> SVN_ERR ((*pset_func) (real_baton, name, value));

Same.

>...
> +++ NEW/trunk/subversion/mod_dav_svn/update.c Tue Jan 8 14:18:22 2002
> @@ -232,10 +232,10 @@
> send_xml(baton->uc, "<D:version-name>%ld</D:version-name>",
> committed_rev);
> send_xml(baton->uc, "<D:creationdate>%s</D:creationdate>",
> - committed_date->data);
> + committed_date ? committed_date->data : "");

This would be:

    if (committed_date != NULL)
        send_xml(...);

>...
> send_xml(baton->uc,
> "<D:creator-displayname>%s</D:creator-displayname>",
> - last_author->data);
> + last_author ? last_author->data : "");

Same.

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 Sat Oct 21 14:36:55 2006

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.