[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: Ramkumar Ramachandra <artagnon_at_gmail.com>
Date: Thu, 19 Aug 2010 02:38:25 +0530

Hi Daniel,

Daniel Shahaf writes:
> > - * @since New in 1.1.
> > + * @since New in 2.0.
>
> s/2.0/1.7/

Fixed.

> > +/**
> > + * Similar to svn_repos_parse_dumpstream3(), but is dumpfile version
> > + * agnostic.
> > + *
>
> Perhaps say: "with @a version_number always set to -1" ?

Fixed, along with Hyrum's suggestions.

> > + /* 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.

It's actually not possible. If I read one line of the dumpstream in
load_editor.c and then call svn_repos_parse_dumpstream2, the function
won't have the version number information because I've already read
that out from the dumpstream, and I can't rewind the stream. Note that
as you already pointed out on IRC, parse_format_version imposes a
maximum already (load.c:647).

> * 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)

Excellent idea; too much work for one patch though- I'll add a TODO
comment about this and re-post the patch.

-- Ram
Received on 2010-08-18 23:11:20 CEST

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