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

Re: [PATCH] New dumpstream parser to check version number

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Wed, 18 Aug 2010 23:39:28 +0300

Ramkumar Ramachandra wrote on Thu, Aug 19, 2010 at 01:29:02 +0530:
> +++ subversion/include/svn_repos.h (working copy)
> @@ -2661,9 +2665,25 @@ typedef svn_repos_parse_fns2_t svn_repos_parser_fn
> * This is enough knowledge to make it easy on vtable implementors,
> * but still allow expansion of the format: most headers are ignored.
> *
> - * @since New in 1.1.
> + * @since New in 2.0.

s/2.0/1.7/

> */
> svn_error_t *
> +svn_repos_parse_dumpstream3(svn_stream_t *stream,
> + const svn_repos_parse_fns2_t *parse_fns,
> + void *parse_baton,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + int version_number,
> + apr_pool_t *pool);
> +
> +/**
> + * Similar to svn_repos_parse_dumpstream3(), but is dumpfile version
> + * agnostic.
> + *

Perhaps say: "with @a version_number always set to -1" ?

> + * @deprecated Provided for backward compatibility with the 1.6 API.
> + */
> +SVN_DEPRECATED
> +svn_error_t *
> svn_repos_parse_dumpstream2(svn_stream_t *stream,
> const svn_repos_parse_fns2_t *parse_fns,
> void *parse_baton,
> Index: subversion/libsvn_repos/load.c
> ===================================================================
> --- subversion/libsvn_repos/load.c (revision 986884)
> +++ subversion/libsvn_repos/load.c (working copy)
> @@ -654,14 +654,27 @@ parse_format_version(const char *versionstring, in
> /* The Main Parser Logic */
> svn_error_t *
> -svn_repos_parse_dumpstream2(svn_stream_t *stream,
> +svn_repos_parse_dumpstream3(svn_stream_t *stream,
> const svn_repos_parse_fns2_t *parse_fns,
> void *parse_baton,
> svn_cancel_func_t cancel_func,
> void *cancel_baton,
> + int version_number,
> apr_pool_t *pool)
> {
> svn_boolean_t eof;
> @@ -690,6 +703,11 @@ svn_error_t *
> return svn_error_createf(SVN_ERR_STREAM_MALFORMED_DATA, NULL,
> _("Unsupported dumpfile version: %d"), version);
>
> + /* Error out if version doesn't match with the provided version_number */
> + if (version_number != -1 && version_number != version)

svnrdump needs an inequality check here. But 'svnadmin load', for
example, needs an "is at most" check. So I don't think this is generic
enough: it would be better to provide something that 'svnadmin load' can
use too.

So, two options:

* Repeat the parse_format_version() trick (from load.c) in svnrdump.

* In the API, PARSE_FNS could grow a dumpfile_version_record() callback
  (like it has a uuid_record()) callback. The caller can implement
  whatever check they want in that callback. (possibly with a special
  error code that svn_repos_parse_dumpstreamN() recognizes; compare
  usage of SVN_ERR_CANCELLED)
Received on 2010-08-18 22:42:29 CEST

This is an archived mail posted to the Subversion Dev mailing list.