You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2011/12/17 01:56:00 UTC
svn commit: r1215374 - /subversion/trunk/subversion/libsvn_ra_neon/log.c
Author: cmpilato
Date: Sat Dec 17 00:56:00 2011
New Revision: 1215374
URL: http://svn.apache.org/viewvc?rev=1215374&view=rev
Log:
Followup to r1215260, just tightening up some loose logic.
* subversion/libsvn_ra_neon/log.c
(log_start_element): Reduce the scope of the check for an encoding
to just those tags for which it makes sense to have one. While
here, fix a long-standing potential segfault (apr_pstrdup on NULL
input).
(maybe_decode_log_cdata): Lose 'pool' parameter, and just store in
the result in lb->subpool.
(log_end_element): Update call to maybe_decode_log_cdata().
Modified:
subversion/trunk/subversion/libsvn_ra_neon/log.c
Modified: subversion/trunk/subversion/libsvn_ra_neon/log.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_neon/log.c?rev=1215374&r1=1215373&r2=1215374&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_neon/log.c (original)
+++ subversion/trunk/subversion/libsvn_ra_neon/log.c Sat Dec 17 00:56:00 2011
@@ -165,20 +165,27 @@ log_start_element(int *elem, void *baton
case ELEM_comment:
lb->want_cdata = lb->cdata;
svn_stringbuf_setempty(lb->cdata);
+ lb->cdata_encoding = NULL;
+
+ /* Some tags might contain encoded CDATA. */
+ if ((elm->id == ELEM_comment) ||
+ (elm->id == ELEM_creator_displayname) ||
+ (elm->id == ELEM_log_date) ||
+ (elm->id == ELEM_rev_prop))
+ {
+ lb->cdata_encoding = svn_xml_get_attr_value("encoding", atts);
+ if (lb->cdata_encoding)
+ lb->cdata_encoding = apr_pstrdup(lb->subpool, lb->cdata_encoding);
+ }
- lb->cdata_encoding = svn_xml_get_attr_value("encoding", atts);
- if (lb->cdata_encoding)
- lb->cdata_encoding = apr_pstrdup(lb->subpool, lb->cdata_encoding);
-
+ /* revprop tags have names. */
if (elm->id == ELEM_revprop)
{
- lb->revprop_name = apr_pstrdup(lb->subpool,
- svn_xml_get_attr_value("name",
- atts));
+ const char *revprop_name = svn_xml_get_attr_value("name", atts);
if (lb->revprop_name == NULL)
return svn_error_createf(SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
_("Missing name attr in revprop element"));
-
+ lb->revprop_name = apr_pstrdup(lb->subpool, revprop_name);
}
break;
case ELEM_has_children:
@@ -246,12 +253,11 @@ log_start_element(int *elem, void *baton
/*
* Set *DECODED_CDATA to a copy of current CDATA being tracked in LB,
- * decoded as necessary, and allocated from POOL.
+ * decoded as necessary, and allocated from LB->subpool.
*/
static svn_error_t *
maybe_decode_log_cdata(const svn_string_t **decoded_cdata,
- struct log_baton *lb,
- apr_pool_t *pool)
+ struct log_baton *lb)
{
if (lb->cdata_encoding)
{
@@ -264,11 +270,11 @@ maybe_decode_log_cdata(const svn_string_
if (strcmp(lb->cdata_encoding, "base64") != 0)
return svn_error_create(SVN_ERR_XML_MALFORMED, NULL, NULL);
- *decoded_cdata = svn_base64_decode_string(&in, pool);
+ *decoded_cdata = svn_base64_decode_string(&in, lb->subpool);
}
else
{
- *decoded_cdata = svn_string_create_from_buf(lb->cdata, pool);
+ *decoded_cdata = svn_string_create_from_buf(lb->cdata, lb->subpool);
}
return SVN_NO_ERROR;
@@ -287,7 +293,7 @@ log_end_element(void *baton, int state,
const svn_string_t *decoded_cdata;
if (lb->want_cdata)
- SVN_ERR(maybe_decode_log_cdata(&decoded_cdata, lb, lb->subpool));
+ SVN_ERR(maybe_decode_log_cdata(&decoded_cdata, lb));
switch (state)
{
Re: svn commit: r1215374 - /subversion/trunk/subversion/libsvn_ra_neon/log.c
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 01/04/2012 08:25 AM, Philip Martin wrote:
> Philip Martin <ph...@wandisco.com> writes:
>
>> This causes log_tests.py 25 to fail with neon when built with -O2 on
>> my Linux box, the test passes when built -O0.
>
> valgrind identified the problem, fixed by r1227146.
>
Thanks, Philip!
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1215374 - /subversion/trunk/subversion/libsvn_ra_neon/log.c
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> This causes log_tests.py 25 to fail with neon when built with -O2 on
> my Linux box, the test passes when built -O0.
valgrind identified the problem, fixed by r1227146.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1215374 - /subversion/trunk/subversion/libsvn_ra_neon/log.c
Posted by Philip Martin <ph...@wandisco.com>.
cmpilato@apache.org writes:
> Author: cmpilato
> Date: Sat Dec 17 00:56:00 2011
> New Revision: 1215374
>
> URL: http://svn.apache.org/viewvc?rev=1215374&view=rev
> Log:
> Followup to r1215260, just tightening up some loose logic.
>
> * subversion/libsvn_ra_neon/log.c
> (log_start_element): Reduce the scope of the check for an encoding
> to just those tags for which it makes sense to have one. While
> here, fix a long-standing potential segfault (apr_pstrdup on NULL
> input).
> (maybe_decode_log_cdata): Lose 'pool' parameter, and just store in
> the result in lb->subpool.
> (log_end_element): Update call to maybe_decode_log_cdata().
This causes log_tests.py 25 to fail with neon when built with -O2 on
my Linux box, the test passes when built -O0. The compiler warns
../src/subversion/libsvn_ra_neon/log.c: In function ‘log_end_element’:
../src/subversion/libsvn_ra_neon/log.c:293: warning: ‘decoded_cdata’ may be used uninitialized in this function
but that may not be the cause since the patch below doesn't fix the
fail:
Index: subversion/libsvn_ra_neon/log.c
===================================================================
--- subversion/libsvn_ra_neon/log.c (revision 1227106)
+++ subversion/libsvn_ra_neon/log.c (working copy)
@@ -292,8 +292,7 @@
struct log_baton *lb = baton;
const svn_string_t *decoded_cdata;
- if (lb->want_cdata)
- SVN_ERR(maybe_decode_log_cdata(&decoded_cdata, lb));
+ SVN_ERR(maybe_decode_log_cdata(&decoded_cdata, lb));
switch (state)
{
--
Philip