[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: Hyrum K. Wright <hyrum_wright_at_mail.utexas.edu>
Date: Wed, 18 Aug 2010 21:33:31 +0100

On Wed, Aug 18, 2010 at 8:59 PM, Ramkumar Ramachandra
<artagnon_at_gmail.com> wrote:
> [[[
> * subversion/libsvn_repos/load.c
>  (svn_repos_parse_dumpstream2, svn_repos_parse_dumpstream3): Rename
>  the older function and add a version_number argument; error out if
>  there's a version mismatch.
>
> * subversion/include/svn_repos.h
>  (svn_repos_parse_dumpstream2, svn_repos_parse_dumpstream3): Add new
>  function and mark svn_repos_parse_dumpstream2 as deprecated.
>
> * subversion/svnrdump/load_editor.c
>  (drive_dumpstream_loader): Update to use the new API, and call it
>  with version_number 3.
> ]]]
>
> Index: subversion/include/svn_repos.h
> ===================================================================
> --- subversion/include/svn_repos.h      (revision 986884)
> +++ subversion/include/svn_repos.h      (working copy)
> @@ -2646,6 +2646,10 @@ typedef svn_repos_parse_fns2_t svn_repos_parser_fn
>  * @a cancel_baton as argument to see if the client wishes to cancel
>  * the dump.
>  *
> + * If @a version_number is -1, it is ignored and the dumpstream is
> + * parsed without this information. If not -1, the function checks the

We try to avoid magic numbers. Is there a #define or constant
somewhere that you could use? If not, I'd add a #define for
UNKNOWN_VERSION or something similar (look at other #define's in the
header files).

> + * dumpstream's version number, and errors out if there's a mismatch.
> + *
>  * This parser has built-in knowledge of the dumpfile format, but only
>  * in a general sense:
>  *
> @@ -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.

Uh, we haven't (not do we have an immediate plans to) released 2.0 yet. :)

>  */
>  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.
> + *
> + * @deprecated Provided for backward compatibility with the 1.6 API.

Should maintain the @since tag from before.

> + */
> +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
>  }
>
>
> +svn_error_t *
> +svn_repos_parse_dumpstream2(svn_stream_t *stream,
> +                            const svn_repos_parse_fns2_t *parse_fns,
> +                            void *parse_baton,
> +                            svn_cancel_func_t cancel_func,
> +                            void *cancel_baton,
> +                            apr_pool_t *pool)
> +{
> +  return svn_repos_parse_dumpstream3(stream, parse_fns, parse_baton,
> +                                     cancel_func, cancel_baton, -1, pool);
> +}
>
> +
>  /* 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)

Another magic number.

> +    return svn_error_createf(SVN_ERR_STREAM_MALFORMED_DATA, NULL,
> +                             _("Unsupported dumpfile version: %d"), version);
> +
>   /* A dumpfile "record" is defined to be a header-block of
>      rfc822-style headers, possibly followed by a content-block.
>
> Index: subversion/svnrdump/load_editor.c
> ===================================================================
> --- subversion/svnrdump/load_editor.c   (revision 986884)
> +++ subversion/svnrdump/load_editor.c   (working copy)
> @@ -540,8 +540,8 @@ drive_dumpstream_loader(svn_stream_t *stream,
>   pb = parse_baton;
>
>   SVN_ERR(svn_ra_get_repos_root2(session, &(pb->root_url), pool));
> -  SVN_ERR(svn_repos_parse_dumpstream2(stream, parser, parse_baton,
> -                                      NULL, NULL, pool));
> +  SVN_ERR(svn_repos_parse_dumpstream3(stream, parser, parse_baton,
> +                                      NULL, NULL, 3, pool));

Another magic number.

>
>   return SVN_NO_ERROR;
>  }
>
Received on 2010-08-18 22:34:30 CEST

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