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 <cmpil...@collab.net>
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to