Re: [PATCH svn-fe 5/7] Fix vcs-svn/fast_export reinit bugs

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Dmitry Ivankov wrote:

> first_commit_done was not set in _init, but it
> is needed if fast_export_ is used/_init-ed twice.

Thanks, good catch.  I wonder if it's possible to detect this kind of
thing automatically --- maybe it would make sense for test-svn-fe to
learn to apply two dumps in sequence so this code could be exercised.

> Same thing for the branch_name.

This one should be squashed with the previous patch for easier review
imho.  New readers never have to know the details of early mistakes.

>
> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
> ---
>  vcs-svn/fast_export.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
> index 8b14b74..6a4360f 100644
> --- a/vcs-svn/fast_export.c
> +++ b/vcs-svn/fast_export.c
> @@ -32,6 +32,8 @@ static int init_postimage(void)
>  
>  void fast_export_init(int fd, const char* branch)
>  {
> +	first_commit_done = 0;
> +	strbuf_reset(&branch_name);
>  	strbuf_addstr(&branch_name, branch);
>  	if (buffer_fdinit(&report_buffer, fd))
>  		die_errno("cannot read from file descriptor %d", fd);
> @@ -45,7 +47,9 @@ void fast_export_deinit(void)
>  
>  void fast_export_reset(void)
>  {
> +	first_commit_done = 0;

I don't think this is needed --- fast_export_reset is called when we
are about to exit.  Maybe the vcs-svn::*_reset functions should be
renamed to foo_free or something similar to avoid confusion
(especially when comparing to strbuf_reset).

>  	buffer_reset(&report_buffer);
> +	strbuf_reset(&branch_name);
>  }
>  
>  void fast_export_delete(const char *path)
> -- 
> 1.7.3.4
>