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
Received on Thu Jan 26 15:35:26 2006