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

Re: svn commit: r30843 - trunk/subversion/libsvn_ra_serf

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Wed, 30 Apr 2008 17:45:07 -0400

lgo_at_tigris.org writes:
> Log:
> ra_serf: Gracefully handle XML parsing errors, instead of aborting.
>
> * subversion/libsvn_ra_serf/replay.c
> (struct replay_context_t): add field parser_ctx. We need this
> reference to get back the XML parsing error.
> (svn_ra_serf__replay_range): correctly handle the first parser error in
> the list of received responses.
>
> --- trunk/subversion/libsvn_ra_serf/replay.c (r30842)
> +++ trunk/subversion/libsvn_ra_serf/replay.c (r30843)
> @@ -116,6 +116,9 @@ typedef struct {
> apr_hash_t *revs_props;
> apr_hash_t *props;
>
> + /* Keep a reference to the XML parser ctx to report any errors. */
> + svn_ra_serf__xml_parser_t *parser_ctx;
> +
> } replay_context_t;
>
>
> @@ -693,6 +696,7 @@ svn_ra_serf__replay_range(svn_ra_session
> svn_ra_serf__list_t *done_list;
> svn_ra_serf__list_t *done_reports = NULL;
> replay_context_t *replay_ctx;
> + int status_code;
>
> /* Send pending requests, if any. Limit the number of outstanding
> requests to MAX_OUTSTANDING_REQUESTS. */
> @@ -746,12 +750,16 @@ svn_ra_serf__replay_range(svn_ra_session
> parser_ctx->start = start_replay;
> parser_ctx->end = end_replay;
> parser_ctx->cdata = cdata_replay;
> + parser_ctx->status_code = &status_code;
> parser_ctx->done = &replay_ctx->done;
> parser_ctx->done_list = &done_reports;
> parser_ctx->done_item = &replay_ctx->done_item;
> handler->response_handler = svn_ra_serf__handle_xml_parser;
> handler->response_baton = parser_ctx;

It might be worth making a comment where we declare 'status_code' that
its sole purpose is to be an address so that the
'parser_ctx->status_code' field will not be NULL (and therefore the
parser will not abort on error). Otherwise, the fact that we carefully
assign status_code's address but never check its value later could be
confusing.

(Or am I misinterpreting what's going on?)

That concern notwithstanding, I've voted for this in 1.5.x/STATUS.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-04-30 23:45:34 CEST

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.