You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2005/11/07 14:42:40 UTC
Re: svn commit: r17225 - in branches/svndiff1: . notes
subversion/include subversion/libsvn_delta subversion/libsvn_fs_base
subversion/libsvn_fs_base/util subversion/libsvn_ra_svn
subversion/libsvn_repos subversion/tests/libsvn_delta
dberlin@tigris.org writes:
> Author: dberlin
> Date: Sun Nov 6 18:33:49 2005
> New Revision: 17225
> @@ -165,7 +215,20 @@
> append_encoded_int (header, window->sview_offset, pool);
> append_encoded_int (header, window->sview_len, pool);
> append_encoded_int (header, window->tview_len, pool);
> + if (err == SVN_NO_ERROR && eb->version == 1)
> + {
> + err = zlib_encode (instructions, i1);
> + instructions = i1;
The error handling looks dodgy. Is instructions = i1 correct if
zlib_encode returns an error?
> + }
> append_encoded_int (header, instructions->len, pool);
Is instructions->len valid if zlib_encode returned an error?
> + if (err == SVN_NO_ERROR && eb->version == 1)
> + {
> + svn_stringbuf_t *temp;
> + temp = svn_stringbuf_create_from_string (window->new_data, pool);
> + err = zlib_encode (temp, newdata);
> + window->new_data = svn_string_create_from_buf (newdata, pool);
Same again, is newdata valid if zlib_encode returned an error?
> + }
> +
> append_encoded_int (header, window->new_data->len, pool);
And here. I suppose zlib_encode might provide a guarantee that makes
these constructs work, but it's not obvious to me.
Also, if there is an error I think it gets leaked here as the next bit
of code overwrites err.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r17225 - in branches/svndiff1: . notes
subversion/include subversion/libsvn_delta subversion/libsvn_fs_base
subversion/libsvn_fs_base/util subversion/libsvn_ra_svn
subversion/libsvn_repos subversion/tests/libsvn_delta
Posted by Daniel Berlin <db...@dberlin.org>.
On Mon, 2005-11-07 at 14:42 +0000, Philip Martin wrote:
> dberlin@tigris.org writes:
>
> > Author: dberlin
> > Date: Sun Nov 6 18:33:49 2005
> > New Revision: 17225
>
> > @@ -165,7 +215,20 @@
> > append_encoded_int (header, window->sview_offset, pool);
> > append_encoded_int (header, window->sview_len, pool);
> > append_encoded_int (header, window->tview_len, pool);
> > + if (err == SVN_NO_ERROR && eb->version == 1)
> > + {
> > + err = zlib_encode (instructions, i1);
> > + instructions = i1;
>
> The error handling looks dodgy. Is instructions = i1 correct if
> zlib_encode returns an error?
Yeah, i forgot to SVN_ERR them, i'll fix it.
>
> > + }
> > append_encoded_int (header, instructions->len, pool);
>
> Is instructions->len valid if zlib_encode returned an error?
>
> > + if (err == SVN_NO_ERROR && eb->version == 1)
> > + {
> > + svn_stringbuf_t *temp;
> > + temp = svn_stringbuf_create_from_string (window->new_data, pool);
> > + err = zlib_encode (temp, newdata);
> > + window->new_data = svn_string_create_from_buf (newdata, pool);
>
> Same again, is newdata valid if zlib_encode returned an error?
Nope.
>
> > + }
> > +
> > append_encoded_int (header, window->new_data->len, pool);
>
> And here. I suppose zlib_encode might provide a guarantee that makes
> these constructs work, but it's not obvious to me.
Nope, we should have returned an error.
>
> Also, if there is an error I think it gets leaked here as the next bit
> of code overwrites err.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org