I don't think this is the correct approach. svn_stringbuf_t is
*designed* to put a NUL at the end of the public length. Thus, it is
supposed to properly manage the +1 inside its functions.
The correct fix is to put a ++minimum_size into svn_stringbuf_ensure()
rather than make callers worry about space for the private NUL
character.
Please revert this commit. Code should not have to compensate for
stringbuf's internal concept.
On Thu, Feb 9, 2012 at 11:52, <julianfoad_at_apache.org> wrote:
> Author: julianfoad
> Date: Thu Feb 9 16:52:52 2012
> New Revision: 1242397
>
> URL: http://svn.apache.org/viewvc?rev=1242397&view=rev
> Log:
> Fix calls to svn_stringbuf_ensure() where the caller was not allowing for a
> terminating NUL.
>
> * subversion/libsvn_subr/skel.c
> (unparse): Allow for terminating NUL. These cases were harmless.
>
> * subversion/libsvn_subr/stream.c
> (stream_readline_chunky): Allow for terminating NUL. It looks like this
> case could potentially have led to undefined behaviour, though this has
> not been confirmed.
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/skel.c
> subversion/trunk/subversion/libsvn_subr/stream.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/skel.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/skel.c?rev=1242397&r1=1242396&r2=1242397&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/skel.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/skel.c Thu Feb 9 16:52:52 2012
> @@ -508,7 +508,7 @@ unparse(const svn_skel_t *skel, svn_stri
>
> /* Make sure we have room for the length, the space, and the
> atom's contents. */
> - svn_stringbuf_ensure(str, str->len + length_len + 1 + skel->len);
> + svn_stringbuf_ensure(str, str->len + length_len + 1 + skel->len + 1);
> svn_stringbuf_appendbytes(str, buf, length_len);
> str->data[str->len++] = ' ';
> svn_stringbuf_appendbytes(str, skel->data, skel->len);
> @@ -520,7 +520,7 @@ unparse(const svn_skel_t *skel, svn_stri
> svn_skel_t *child;
>
> /* Emit an opening parenthesis. */
> - svn_stringbuf_ensure(str, str->len + 1);
> + svn_stringbuf_ensure(str, str->len + 1 + 1);
> str->data[str->len++] = '(';
>
> /* Append each element. Emit a space between each pair of elements. */
> @@ -529,7 +529,7 @@ unparse(const svn_skel_t *skel, svn_stri
> unparse(child, str, pool);
> if (child->next)
> {
> - svn_stringbuf_ensure(str, str->len + 1);
> + svn_stringbuf_ensure(str, str->len + 1 + 1);
> str->data[str->len++] = ' ';
> }
> }
>
> Modified: subversion/trunk/subversion/libsvn_subr/stream.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/stream.c?rev=1242397&r1=1242396&r2=1242397&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/stream.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/stream.c Thu Feb 9 16:52:52 2012
> @@ -376,7 +376,7 @@ stream_readline_chunky(svn_stringbuf_t *
> {
> /* Append the next chunk to the string read so far.
> */
> - svn_stringbuf_ensure(str, str->len + LINE_CHUNK_SIZE);
> + svn_stringbuf_ensure(str, str->len + LINE_CHUNK_SIZE + 1);
> numbytes = LINE_CHUNK_SIZE;
> SVN_ERR(svn_stream_read(stream, str->data + str->len, &numbytes));
> str->len += numbytes;
>
>
Received on 2012-02-09 18:07:24 CET