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

["Peter N. Lundblad" <pe...@famlundblad.se>]

Here are some more or less minor nits:-)

On Wed, 25 Jan 2006 jerenkrantz@tigris.org wrote:

> Modified: trunk/subversion/libsvn_ra_serf/serf.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_ra_serf/serf.c?rev=18244&p1=trunk/subversion/libsvn_ra_serf/serf.c&p2=trunk/subversion/libsvn_ra_serf/serf.c&r1=18243&r2=18244
> ==============================================================================
> --- trunk/subversion/libsvn_ra_serf/serf.c	(original)
> +++ trunk/subversion/libsvn_ra_serf/serf.c	Wed Jan 25 21:58:18 2006
> @@ -154,12 +163,372 @@
>    abort();
>  }
>
> +typedef struct {
> +  const char *namespace;
> +  const char *name;
> +} dav_props_t;
> +
Please document new structs.


> +static const char *
> +fetch_prop (apr_hash_t *props,
> +            const char *path,
> +            const char *ns,
> +            const char *name)

...and functions.

> +
> +static void
> +set_prop (apr_hash_t *props, const char *path,
           ^
Bah, there I caught you:-)

> +static serf_bucket_t*

Space before *.

> +static apr_status_t
> +handle_propfind (serf_bucket_t *response,
> +                 void *handler_baton,
> +                 apr_pool_t *pool)


Does this implement some callback signature in serf? Good to note which
one in that case.

> +{
> +  const char *data;
> +  apr_size_t len;
> +  serf_status_line sl;
> +  apr_status_t status;
> +  propfind_context_t *ctx = handler_baton;
> +
> +  status = serf_bucket_response_status(response, &sl);
> +  if (status)
> +    {
> +      if (APR_STATUS_IS_EAGAIN(status))
> +        {
> +          return APR_SUCCESS;
> +        }
> +      abort();
> +    }
> +
> +  while (1)
> +    {
> +      status = serf_bucket_read(response, 2048, &data, &len);
> +      if (SERF_BUCKET_READ_ERROR(status))
> +        {
> +          return status;
> +        }
> +
> +      /* parse the response
> +       *  <?xml version="1.0" encoding="utf-8"?>
> +       *  <D:multistatus xmlns:D="DAV:" xmlns:ns1="http://subversion.tigris.org/xmlns/dav/" xmlns:ns0="DAV:">
> +       *    <D:response xmlns:lp1="DAV:" xmlns:lp3="http://subversion.tigris.org/xmlns/dav/">
> +       *      <D:href>/repos/projects/serf/trunk/</D:href>
> +       *      <D:propstat>
> +       *        <D:prop>
> +       *          <lp1:version-controlled-configuration>
> +       *            <D:href>/repos/projects/!svn/vcc/default</D:href>
> +       *          </lp1:version-controlled-configuration>
> +       *          <lp1:resourcetype>
> +       *            <D:collection/>
> +       *          </lp1:resourcetype>
> +       *          <lp3:baseline-relative-path>
> +       *            serf/trunk
> +       *          </lp3:baseline-relative-path>
> +       *          <lp3:repository-uuid>
> +       *            61a7d7f5-40b7-0310-9c16-bb0ea8cb1845
> +       *          </lp3:repository-uuid>
> +       *        </D:prop>
> +       *        <D:status>HTTP/1.1 200 OK</D:status>
> +       *      </D:propstat>
> +       *    </D:response>
> +       *  </D:multistatus>
> +       */
> +      fwrite(data, 1, len, stdout);
> +
> +      if (APR_STATUS_IS_EOF(status))
> +        {
> +          ctx->done = 1;

s/1/TRUE/

...
> +  /* Create the propfind context. */
> +  prop_ctx = apr_pcalloc(pool, sizeof(*prop_ctx));
> +  prop_ctx->pool = pool;
> +  prop_ctx->path = url;
> +  prop_ctx->find_props = props;
> +  prop_ctx->ret_props = ret_props;
> +  prop_ctx->done = 0;
s/0/FALSE/, or drop the line, since it's redundant.

> +
> +  serf_request_deliver(request, req_bkt,
> +                       accept_response, sess,
> +                       handle_propfind, prop_ctx);
> +
> +  while (!prop_ctx->done)
> +    {
> +      status = serf_context_run(sess->context, SERF_DURATION_FOREVER, pool);
> +      if (APR_STATUS_IS_TIMEUP(status))
> +        {
> +          continue;
> +        }
> +      if (status)
> +        {
> +          return svn_error_wrap_apr(status, "Error retrieving PROPFIND");

Missing _() around the message.

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org