Few stylistic comments, please ignore if it does not make sense.
> +static date_info_t *
> +push_state(svn_ra_serf__xml_parser_t *parser,
> + date_context_t *date_ctx,
>
Unused argument 'date_ctx'.
> + date_state_e state)
> +{
> + svn_ra_serf__xml_push_state(parser, state);
> +
> + if (state == VERSION_NAME)
> + {
>
As it is called only once(and being a static function) with
state==VERSION_NAME always why to do this check again? Or this code is
futuristic?
> + date_info_t *info;
> +
> + info = apr_palloc(parser->state->pool, sizeof(*info));
> +
> + info->tmp = NULL;
> + info->tmp_len = 0;
>
Why not apr_pcalloc?
> +
> +static svn_error_t *
> +start_getdate(svn_ra_serf__xml_parser_t *parser,
> + void *userData,
> + svn_ra_serf__dav_props_t name,
> + const char **attrs)
> +{
> + date_context_t *date_ctx = userData;
>
If push_state's state is changed we don't need this date_ctx.
> + date_state_e state;
> +
> + state = parser->state->current_state;
> +
> + if (state == NONE &&
> + strcmp(name.name, "version-name") == 0)
> + {
> + push_state(parser, date_ctx, VERSION_NAME);
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> +static svn_error_t *
> +end_getdate(svn_ra_serf__xml_parser_t *parser,
> + void *userData,
> + svn_ra_serf__dav_props_t name)
>
Arguments need to be aligned.
> +{
> +
> +static svn_error_t *
> +cdata_getdate(svn_ra_serf__xml_parser_t *parser,
> + void *userData,
> + const char *data,
> + apr_size_t len)
> +{
> + date_context_t *date_ctx = userData;
>
unused variable date_ctx.
> +
> +svn_error_t *
> +svn_ra_serf__get_dated_revision(svn_ra_session_t *ra_session,
> + svn_revnum_t *revision,
> + apr_time_t tm,
> + apr_pool_t *pool)
> +{
> + date_context_t *date_ctx;
> + svn_ra_serf__session_t *session = ra_session->priv;
> + svn_ra_serf__handler_t *handler;
> + svn_ra_serf__xml_parser_t *parser_ctx;
> + serf_bucket_t *buckets, *tmp;
>
Unused variables buckets and tmp.
> + const char *vcc_url;
> + int status_code;
> +
>
> +
> + svn_ra_serf__request_create(handler);
> +
> + *date_ctx->revision = SVN_INVALID_REVNUM;
>
Why to set with 'SVN_INVALID_REVNUM' as we don't use date_ctx down the
line and is not visible to caller?
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Nov 1 10:39:55 2006