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

Re: svn commit: r17232 - branches/svndiff1/subversion/libsvn_delta

From: Daniel Berlin <dberlin_at_dberlin.org>
Date: 2005-11-07 19:01:48 CET

On Mon, 2005-11-07 at 17:54 +0000, Philip Martin wrote:
> dberlin@tigris.org writes:
>
> > Author: dberlin
> > Date: Mon Nov 7 09:06:38 2005
> > New Revision: 17232
>
> > --- branches/svndiff1/subversion/libsvn_delta/svndiff.c (original)
> > +++ branches/svndiff1/subversion/libsvn_delta/svndiff.c Mon Nov 7 09:06:38 2005
> > @@ -217,7 +217,7 @@
> > append_encoded_int (header, window->tview_len, pool);
> > if (err == SVN_NO_ERROR && eb->version == 1)
>
> Looking at the whole function I see that err is initialised but not
> otherwise used before this conditional, so it will always have the
> value SVN_NO_ERROR here and in the next conditional.
>
> The err variable was present before your change, but I'm not sure why
> it exists. Is there some reason that the statements that set err, the
> calls to svn_stream_write, don't use SVN_ERR? Perhaps it's important
> that the svn_destroy_pool line at the end of the function gets
> executed even when an error occurs? If so then your use of SVN_ERR
> might be wrong since it will bypass that line. There is a similar
> problem with the SVN_ERR that occurs near the start of the function,
> but that might be a bug as well.

Hmmmmmmmmmmmmmmmmmmmm.
I think i'm just going to replace this with some gotos, since that seems
the cleanest way to ensure the pool gets destroyed, but that the correct
value of err is there.
Unless you've got a better idea.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Nov 7 19:03:08 2005

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