You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2012/05/13 07:33:02 UTC

svn commit: r1337781 - /subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c

Author: gstein
Date: Sun May 13 05:33:01 2012
New Revision: 1337781

URL: http://svn.apache.org/viewvc?rev=1337781&view=rev
Log:
Switch get-location-segments over to v2 xml parsing.

* subversion/libsvn_ra_serf/getlocationsegments.c:
  (gls_context_t): remove SUBPOOL, INSIDE_REPORT, DONE
  (INITIAL, REPORT, SEGMENT): add enum for parse states
  (gls_ttable): new transition table
  (start_gls, end_gls): removed; obsolete.
  (gls_closed): new close handler. call the receiver with the segment
    information.
  (svn_ra_serf__get_location_segments): drop the old parsing stuff.
    add the new v2 parsing. use run_one(). remove the detection of
    ending within the report element.

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c

Modified: subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c?rev=1337781&r1=1337780&r2=1337781&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c Sun May 13 05:33:01 2012
@@ -49,80 +49,60 @@ typedef struct gls_context_t {
   svn_location_segment_receiver_t receiver;
   void *receiver_baton;
 
-  /* subpool used only as long as a single receiver invocation */
-  apr_pool_t *subpool;
-
-  /* True iff we're looking at a child of the outer report tag */
-  svn_boolean_t inside_report;
-
-  svn_boolean_t done;
 } gls_context_t;
 
+enum {
+  INITIAL = 0,
+  REPORT,
+  SEGMENT
+};
+
+#define D_ "DAV:"
+#define S_ SVN_XML_NAMESPACE
+static const svn_ra_serf__xml_transition_t gls_ttable[] = {
+  { INITIAL, S_, "get-location-segments-report", REPORT,
+    FALSE, { NULL }, FALSE, FALSE },
 
-static svn_error_t *
-start_gls(svn_ra_serf__xml_parser_t *parser,
-          svn_ra_serf__dav_props_t name,
-          const char **attrs,
-          apr_pool_t *scratch_pool)
-{
-  gls_context_t *gls_ctx = parser->user_data;
+  { REPORT, S_, "location-segment", SEGMENT,
+    FALSE, { "?path", "range-start", "range-end", NULL }, FALSE, TRUE },
 
-  if ((! gls_ctx->inside_report)
-      && strcmp(name.name, "get-location-segments-report") == 0)
-    {
-      gls_ctx->inside_report = TRUE;
-    }
-  else if (gls_ctx->inside_report
-           && strcmp(name.name, "location-segment") == 0)
-    {
-      const char *rev_str;
-      svn_revnum_t range_start = SVN_INVALID_REVNUM;
-      svn_revnum_t range_end = SVN_INVALID_REVNUM;
-      const char *path = NULL;
-
-      path = svn_xml_get_attr_value("path", attrs);
-      rev_str = svn_xml_get_attr_value("range-start", attrs);
-      if (rev_str)
-        range_start = SVN_STR_TO_REV(rev_str);
-      rev_str = svn_xml_get_attr_value("range-end", attrs);
-      if (rev_str)
-        range_end = SVN_STR_TO_REV(rev_str);
-
-      if (SVN_IS_VALID_REVNUM(range_start) && SVN_IS_VALID_REVNUM(range_end))
-        {
-          svn_location_segment_t *segment = apr_pcalloc(gls_ctx->subpool,
-                                                        sizeof(*segment));
-          segment->path = path;
-          segment->range_start = range_start;
-          segment->range_end = range_end;
-          SVN_ERR(gls_ctx->receiver(segment,
-                                    gls_ctx->receiver_baton,
-                                    gls_ctx->subpool));
-          svn_pool_clear(gls_ctx->subpool);
-        }
-      else
-        {
-          return svn_error_create(SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
-                                  _("Expected valid revision range"));
-        }
-    }
+  { 0 }
+};
 
-  return SVN_NO_ERROR;
-}
 
+/* Conforms to svn_ra_serf__xml_closed_t  */
 static svn_error_t *
-end_gls(svn_ra_serf__xml_parser_t *parser,
-        svn_ra_serf__dav_props_t name,
-        apr_pool_t *scratch_pool)
+gls_closed(svn_ra_serf__xml_estate_t *xes,
+           void *baton,
+           int leaving_state,
+           const svn_string_t *cdata,
+           apr_hash_t *attrs,
+           apr_pool_t *scratch_pool)
 {
-  gls_context_t *gls_ctx = parser->user_data;
-
-  if (strcmp(name.name, "get-location-segments-report") == 0)
-    gls_ctx->inside_report = FALSE;
+  gls_context_t *gls_ctx = baton;
+  const char *path;
+  const char *start_str;
+  const char *end_str;
+  svn_location_segment_t segment;
+
+  SVN_ERR_ASSERT(leaving_state == SEGMENT);
+
+  path = apr_hash_get(attrs, "path", APR_HASH_KEY_STRING);
+  start_str = apr_hash_get(attrs, "range-start", APR_HASH_KEY_STRING);
+  end_str = apr_hash_get(attrs, "range-end", APR_HASH_KEY_STRING);
+
+  /* The transition table said these must exist.  */
+  SVN_ERR_ASSERT(start_str && end_str);
+
+  segment.path = path;  /* may be NULL  */
+  segment.range_start = SVN_STR_TO_REV(start_str);
+  segment.range_end = SVN_STR_TO_REV(end_str);
+  SVN_ERR(gls_ctx->receiver(&segment, gls_ctx->receiver_baton, scratch_pool));
 
   return SVN_NO_ERROR;
 }
 
+
 /* Implements svn_ra_serf__request_body_delegate_t */
 static svn_error_t *
 create_gls_body(serf_bucket_t **body_bkt,
@@ -179,7 +159,7 @@ svn_ra_serf__get_location_segments(svn_r
   gls_context_t *gls_ctx;
   svn_ra_serf__session_t *session = ra_session->priv;
   svn_ra_serf__handler_t *handler;
-  svn_ra_serf__xml_parser_t *parser_ctx;
+  svn_ra_serf__xml_context_t *xmlctx;
   const char *req_url;
   svn_error_t *err;
 
@@ -190,18 +170,17 @@ svn_ra_serf__get_location_segments(svn_r
   gls_ctx->end_rev = end_rev;
   gls_ctx->receiver = receiver;
   gls_ctx->receiver_baton = receiver_baton;
-  gls_ctx->subpool = svn_pool_create(pool);
-  gls_ctx->inside_report = FALSE;
-  gls_ctx->done = FALSE;
 
   SVN_ERR(svn_ra_serf__get_stable_url(&req_url, NULL /* latest_revnum */,
                                       session, NULL /* conn */,
                                       NULL /* url */, peg_revision,
                                       pool, pool));
 
-  handler = apr_pcalloc(pool, sizeof(*handler));
+  xmlctx = svn_ra_serf__xml_context_create(gls_ttable,
+                                           NULL, gls_closed, gls_ctx,
+                                           pool);
+  handler = svn_ra_serf__create_expat_handler(xmlctx, pool);
 
-  handler->handler_pool = pool;
   handler->method = "REPORT";
   handler->path = req_url;
   handler->body_delegate = create_gls_body;
@@ -210,25 +189,7 @@ svn_ra_serf__get_location_segments(svn_r
   handler->conn = session->conns[0];
   handler->session = session;
 
-  parser_ctx = apr_pcalloc(pool, sizeof(*parser_ctx));
-
-  parser_ctx->pool = pool;
-  parser_ctx->user_data = gls_ctx;
-  parser_ctx->start = start_gls;
-  parser_ctx->end = end_gls;
-  parser_ctx->done = &gls_ctx->done;
-
-  handler->response_handler = svn_ra_serf__handle_xml_parser;
-  handler->response_baton = parser_ctx;
-
-  svn_ra_serf__request_create(handler);
-
-  err = svn_ra_serf__context_run_wait(&gls_ctx->done, session, pool);
-
-  if (gls_ctx->inside_report)
-    err = svn_error_createf(SVN_ERR_RA_DAV_REQUEST_FAILED, err,
-                            _("Location segment report failed on '%s'@'%ld'"),
-                              path, peg_revision);
+  err = svn_ra_serf__context_run_one(handler, pool);
 
   err = svn_error_compose_create(
          svn_ra_serf__error_on_status(handler->sline.code,
@@ -236,8 +197,6 @@ svn_ra_serf__get_location_segments(svn_r
                                       handler->location),
          err);
 
-  svn_pool_destroy(gls_ctx->subpool);
-
   if (err && (err->apr_err == SVN_ERR_UNSUPPORTED_FEATURE))
     return svn_error_create(SVN_ERR_RA_NOT_IMPLEMENTED, err, NULL);
 



RE: svn commit: r1337781 - /subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c

Posted by Greg Stein <gs...@gmail.com>.
Thanks, Bert. I'll take a look in a little while.
On May 13, 2012 11:26 AM, "Bert Huijben" <be...@qqmail.nl> wrote:

>
>
> > -----Original Message-----
> > From: gstein@apache.org [mailto:gstein@apache.org]
> > Sent: zondag 13 mei 2012 7:33
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1337781 -
> > /subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c
> >
> > Author: gstein
> > Date: Sun May 13 05:33:01 2012
> > New Revision: 1337781
> >
> > URL: http://svn.apache.org/viewvc?rev=1337781&view=rev
> > Log:
> > Switch get-location-segments over to v2 xml parsing.
> >
> > * subversion/libsvn_ra_serf/getlocationsegments.c:
> >   (gls_context_t): remove SUBPOOL, INSIDE_REPORT, DONE
> >   (INITIAL, REPORT, SEGMENT): add enum for parse states
> >   (gls_ttable): new transition table
> >   (start_gls, end_gls): removed; obsolete.
> >   (gls_closed): new close handler. call the receiver with the segment
> >     information.
> >   (svn_ra_serf__get_location_segments): drop the old parsing stuff.
> >     add the new v2 parsing. use run_one(). remove the detection of
> >     ending within the report element.
>
> The serf/Win32 buildbot currently reports two test failures:
> FAIL:  basic_tests.py 32: info on file not existing in HEAD
> FAIL:  log_tests.py 33: log with unrelated peg and operative rev targets
>
> Both appear to be caused by a leaked error generated from a server report
> via util.c line 902 (called from svn_ra_serf__expect_empty_body()) for
> svn_ra_serf__get_locations() calls.
>
> The serf call currently returns success, while it probably should have
> reported this error.
>
> (You might have fixed an old error as I remember that there used to be
> some behavior difference between ra implementations here)
>
>
> [I haven't tracked this issue to this specific revision. But this appears
> to be the most relevant in the batch]
>
>        Bert
>
>

Re: svn commit: r1337781 - /subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c

Posted by Greg Stein <gs...@gmail.com>.
On Sun, May 13, 2012 at 5:26 PM, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: gstein@apache.org [mailto:gstein@apache.org]
>> Sent: zondag 13 mei 2012 7:33
>> To: commits@subversion.apache.org
>> Subject: svn commit: r1337781 -
>> /subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c
>>
>> Author: gstein
>> Date: Sun May 13 05:33:01 2012
>> New Revision: 1337781
>>
>> URL: http://svn.apache.org/viewvc?rev=1337781&view=rev
>> Log:
>> Switch get-location-segments over to v2 xml parsing.
>>
>> * subversion/libsvn_ra_serf/getlocationsegments.c:
>>   (gls_context_t): remove SUBPOOL, INSIDE_REPORT, DONE
>>   (INITIAL, REPORT, SEGMENT): add enum for parse states
>>   (gls_ttable): new transition table
>>   (start_gls, end_gls): removed; obsolete.
>>   (gls_closed): new close handler. call the receiver with the segment
>>     information.
>>   (svn_ra_serf__get_location_segments): drop the old parsing stuff.
>>     add the new v2 parsing. use run_one(). remove the detection of
>>     ending within the report element.
>
> The serf/Win32 buildbot currently reports two test failures:
> FAIL:  basic_tests.py 32: info on file not existing in HEAD
> FAIL:  log_tests.py 33: log with unrelated peg and operative rev targets
>
> Both appear to be caused by a leaked error generated from a server report via util.c line 902 (called from svn_ra_serf__expect_empty_body()) for svn_ra_serf__get_locations() calls.
>
> The serf call currently returns success, while it probably should have reported this error.

Yup. It was an error that came from the server. No client-side error
occurred, so success was returned. We now look for potential server
errors on most responses. (in process on getting all of them to
look...)

>
> (You might have fixed an old error as I remember that there used to be some behavior difference between ra implementations here)

Fixed in 1338128. Thanks!

Cheers,
-g

RE: svn commit: r1337781 - /subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: gstein@apache.org [mailto:gstein@apache.org]
> Sent: zondag 13 mei 2012 7:33
> To: commits@subversion.apache.org
> Subject: svn commit: r1337781 -
> /subversion/trunk/subversion/libsvn_ra_serf/getlocationsegments.c
> 
> Author: gstein
> Date: Sun May 13 05:33:01 2012
> New Revision: 1337781
> 
> URL: http://svn.apache.org/viewvc?rev=1337781&view=rev
> Log:
> Switch get-location-segments over to v2 xml parsing.
> 
> * subversion/libsvn_ra_serf/getlocationsegments.c:
>   (gls_context_t): remove SUBPOOL, INSIDE_REPORT, DONE
>   (INITIAL, REPORT, SEGMENT): add enum for parse states
>   (gls_ttable): new transition table
>   (start_gls, end_gls): removed; obsolete.
>   (gls_closed): new close handler. call the receiver with the segment
>     information.
>   (svn_ra_serf__get_location_segments): drop the old parsing stuff.
>     add the new v2 parsing. use run_one(). remove the detection of
>     ending within the report element.

The serf/Win32 buildbot currently reports two test failures:
FAIL:  basic_tests.py 32: info on file not existing in HEAD
FAIL:  log_tests.py 33: log with unrelated peg and operative rev targets

Both appear to be caused by a leaked error generated from a server report via util.c line 902 (called from svn_ra_serf__expect_empty_body()) for svn_ra_serf__get_locations() calls.

The serf call currently returns success, while it probably should have reported this error.

(You might have fixed an old error as I remember that there used to be some behavior difference between ra implementations here)


[I haven't tracked this issue to this specific revision. But this appears to be the most relevant in the batch]

	Bert