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 2012/06/08 18:00:26 UTC
svn commit: r1348130 - /subversion/trunk/subversion/libsvn_ra_serf/util.c
Author: cmpilato
Date: Fri Jun 8 16:00:26 2012
New Revision: 1348130
URL: http://svn.apache.org/viewvc?rev=1348130&view=rev
Log:
* subversion/libsvn_ra_serf/util.c
(start_xml, end_xml, cdata_xml): Wrapper APR_EOF errors returned
from the editor implementations with SVN_ERR_RA_SERF_WRAPPED_ERROR
so those errors can be distinguished from EOF's in the *network
stream* by higher layers.
Modified:
subversion/trunk/subversion/libsvn_ra_serf/util.c
Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1348130&r1=1348129&r2=1348130&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Fri Jun 8 16:00:26 2012
@@ -1200,6 +1200,7 @@ start_xml(void *userData, const char *ra
svn_ra_serf__xml_parser_t *parser = userData;
svn_ra_serf__dav_props_t name;
apr_pool_t *scratch_pool;
+ svn_error_t *err;
if (parser->error)
return;
@@ -1214,7 +1215,11 @@ start_xml(void *userData, const char *ra
svn_ra_serf__expand_ns(&name, parser->state->ns_list, raw_name);
- parser->error = parser->start(parser, name, attrs, scratch_pool);
+ err = parser->start(parser, name, attrs, scratch_pool);
+ if (err && APR_STATUS_IS_EOF(err->apr_err))
+ err = svn_error_create(SVN_ERR_RA_SERF_WRAPPED_ERROR, err, NULL);
+
+ parser->error = err;
}
@@ -1224,6 +1229,7 @@ end_xml(void *userData, const char *raw_
{
svn_ra_serf__xml_parser_t *parser = userData;
svn_ra_serf__dav_props_t name;
+ svn_error_t *err;
apr_pool_t *scratch_pool;
if (parser->error)
@@ -1234,7 +1240,11 @@ end_xml(void *userData, const char *raw_
svn_ra_serf__expand_ns(&name, parser->state->ns_list, raw_name);
- parser->error = parser->end(parser, name, scratch_pool);
+ err = parser->end(parser, name, scratch_pool);
+ if (err && APR_STATUS_IS_EOF(err->apr_err))
+ err = svn_error_create(SVN_ERR_RA_SERF_WRAPPED_ERROR, err, NULL);
+
+ parser->error = err;
}
@@ -1243,6 +1253,7 @@ static void
cdata_xml(void *userData, const char *data, int len)
{
svn_ra_serf__xml_parser_t *parser = userData;
+ svn_error_t *err;
apr_pool_t *scratch_pool;
if (parser->error)
@@ -1254,7 +1265,11 @@ cdata_xml(void *userData, const char *da
/* ### get a real scratch_pool */
scratch_pool = parser->state->pool;
- parser->error = parser->cdata(parser, data, len, scratch_pool);
+ err = parser->cdata(parser, data, len, scratch_pool);
+ if (err && APR_STATUS_IS_EOF(err->apr_err))
+ err = svn_error_create(SVN_ERR_RA_SERF_WRAPPED_ERROR, err, NULL);
+
+ parser->error = err;
}
/* Flip the requisite bits in CTX to indicate that processing of the
Re: svn commit: r1348130 - /subversion/trunk/subversion/libsvn_ra_serf/util.c
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Tuesday, June 12, 2012, C. Michael Pilato wrote:
> On 06/10/2012 12:16 AM, Bert Huijben wrote:
> >> @@ -1214,7 +1215,11 @@ start_xml(void *userData, const char *ra
> >>
> >> svn_ra_serf__expand_ns(&name, parser->state->ns_list, raw_name);
> >>
> >> - parser->error = parser->start(parser, name, attrs, scratch_pool);
> >> + err = parser->start(parser, name, attrs, scratch_pool);
> >> + if (err && APR_STATUS_IS_EOF(err->apr_err))
> >
> > I know it is ugly but maybe you should use !SERF_BUCKET_READ_ERROR()
> instead of just APR_EOF. This should handle all specialized serf errors.
>
> Makes sense.
If it gives you any solace, Johan and I were up to 1am debugging his virus
scanning issue and it is almost certainly another instance of the EAGAIN
being ignored. =) -- justin
Re: svn commit: r1348130 - /subversion/trunk/subversion/libsvn_ra_serf/util.c
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 06/12/2012 04:34 AM, C. Michael Pilato wrote:
> On 06/10/2012 12:16 AM, Bert Huijben wrote:
>>> @@ -1214,7 +1215,11 @@ start_xml(void *userData, const char *ra
>>>
>>> svn_ra_serf__expand_ns(&name, parser->state->ns_list, raw_name);
>>>
>>> - parser->error = parser->start(parser, name, attrs, scratch_pool);
>>> + err = parser->start(parser, name, attrs, scratch_pool);
>>> + if (err && APR_STATUS_IS_EOF(err->apr_err))
>>
>> I know it is ugly but maybe you should use !SERF_BUCKET_READ_ERROR() instead of just APR_EOF. This should handle all specialized serf errors.
>
> Makes sense.
Done in r1349203.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: svn commit: r1348130 - /subversion/trunk/subversion/libsvn_ra_serf/util.c
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 06/10/2012 12:16 AM, Bert Huijben wrote:
>> @@ -1214,7 +1215,11 @@ start_xml(void *userData, const char *ra
>>
>> svn_ra_serf__expand_ns(&name, parser->state->ns_list, raw_name);
>>
>> - parser->error = parser->start(parser, name, attrs, scratch_pool);
>> + err = parser->start(parser, name, attrs, scratch_pool);
>> + if (err && APR_STATUS_IS_EOF(err->apr_err))
>
> I know it is ugly but maybe you should use !SERF_BUCKET_READ_ERROR() instead of just APR_EOF. This should handle all specialized serf errors.
Makes sense.
<rant>
By the way, I hate that macro. It confuses me every single time I see it.
I suspect that when introduced it was intended to be SERF_BUCKET_REAL_ERROR
(note the 'L') and was simply fat-fingered. After all, the docstring claims
that it checks whether a *real* error has occurred, pointing out that, in
fact, specific *read* errors are normal and therefore *not* interesting errors:
/**
* Check whether a real error occurred. Note that bucket read functions
* can return EOF and EAGAIN as part of their "normal" operation, so they
* should not be considered an error.
*/
#define SERF_BUCKET_READ_ERROR(status) ((status) \
&& !APR_STATUS_IS_EOF(status) \
&& !APR_STATUS_IS_EAGAIN(status) \
&& (SERF_ERROR_WAIT_CONN != status))
</rant>
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
RE: svn commit: r1348130 - /subversion/trunk/subversion/libsvn_ra_serf/util.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: cmpilato@apache.org [mailto:cmpilato@apache.org]
> Sent: vrijdag 8 juni 2012 18:00
> To: commits@subversion.apache.org
> Subject: svn commit: r1348130 -
> /subversion/trunk/subversion/libsvn_ra_serf/util.c
>
> Author: cmpilato
> Date: Fri Jun 8 16:00:26 2012
> New Revision: 1348130
>
> URL: http://svn.apache.org/viewvc?rev=1348130&view=rev
> Log:
> * subversion/libsvn_ra_serf/util.c
> (start_xml, end_xml, cdata_xml): Wrapper APR_EOF errors returned
> from the editor implementations with
> SVN_ERR_RA_SERF_WRAPPED_ERROR
> so those errors can be distinguished from EOF's in the *network
> stream* by higher layers.
>
> Modified:
> subversion/trunk/subversion/libsvn_ra_serf/util.c
>
> Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/
> util.c?rev=1348130&r1=1348129&r2=1348130&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/util.c Fri Jun 8 16:00:26
> 2012
> @@ -1200,6 +1200,7 @@ start_xml(void *userData, const char *ra
> svn_ra_serf__xml_parser_t *parser = userData;
> svn_ra_serf__dav_props_t name;
> apr_pool_t *scratch_pool;
> + svn_error_t *err;
>
> if (parser->error)
> return;
> @@ -1214,7 +1215,11 @@ start_xml(void *userData, const char *ra
>
> svn_ra_serf__expand_ns(&name, parser->state->ns_list, raw_name);
>
> - parser->error = parser->start(parser, name, attrs, scratch_pool);
> + err = parser->start(parser, name, attrs, scratch_pool);
> + if (err && APR_STATUS_IS_EOF(err->apr_err))
I know it is ugly but maybe you should use !SERF_BUCKET_READ_ERROR() instead of just APR_EOF. This should handle all specialized serf errors.
Maybe we can hide the ugliness by wrapping it as SVN_RA_SERF__SHOULD_SPECIALIZE_ERROR() or something.
(I know I did use this macro in some earlier serf work)
Bert