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

RE: Re: svn commit: r35974 - in trunk/subversion: include libsvn_subr

From: Chris Foote <cafoote_at_yahoo.com>
Date: Thu, 19 Feb 2009 05:41:29 -0800 (PST)

> How is it inconsistent?
>
It is inconsistent because the svn_stringbuf_ensure API doesn't add one to the size.

/** Make sure that the string @a str has at least @a minimum_size bytes of
 * space available in the memory block.
 *
 * (@a minimum_size should include space for the terminating NULL character.)
 */
void
svn_stringbuf_ensure(svn_stringbuf_t *str, apr_size_t minimum_size);

> The callers "should" have passed at least 1, but the simple fact is
> that they DID NOT. Thus, we had failures. It is much simpler and safer
> to handle that situation, than to audit all callers (which might even
> be in third-party code!).

The svn_stringbuf_create_ensure function is new in 1.6, so there aren't that many callers to audit.

./subversion/libsvn_ra_serf/update.c:1255:
./subversion/libsvn_subr/io.c:1723:
./subversion/libsvn_subr/path.c:777:
./subversion/libsvn_subr/path.c:903:
./subversion/libsvn_subr/path.c:1083:
./subversion/libsvn_subr/stream.c:177:
./subversion/libsvn_subr/stream.c:1071:
./subversion/libsvn_subr/svn_string.c:238:
./subversion/libsvn_subr/svn_string.c:252:
./subversion/libsvn_subr/utf.c:455:
./subversion/libsvn_subr/xml.c:162:
./subversion/libsvn_subr/xml.c:572:

Third-party code that pass 0 as the minimum_size would have been violating the API before Berts change, so they already needed fixing.

Regards,
Chris
>
> Cheers,
> -g
>
> On Thu, Feb 19, 2009 at 13:40, Chris Foote <cafoote_at_yahoo.com> wrote:
> > Hi Bert,
> >
> > Now this API is inconsistent with svn_stringbuf_ensure.
> >
> > IMHO, the callers of svn_stringbuf_create_ensure need to always pass at least 1 for the minimum_size.
> >
> > Regards,
> > Chris
> >
> >> Author: rhuijben
> >> Date: Thu Feb 19 03:41:36 2009
> >> New Revision: 35974
> >>
> >> Log:
> >> Following up on r35968, fix the exact svn_stringbuf_create block size
> >> behavior tested by lt-string-test 10: block initialization and growth.
> >>
> >> * subversion/include/svn_string.h
> >> (svn_stringbuf_create_ensure): Update comment to document the +1 behavior.
> >> * subversion/libsvn_subr/svn_string.c
> >> (svn_stringbuf_ncreate): Stop incrementing the buffer size as
> >> svn_stringbuf_create_ensure does this for us.
> >>
> >> Modified:
> >> trunk/subversion/include/svn_string.h
> >> trunk/subversion/libsvn_subr/svn_string.c
> >>
> >> Modified: trunk/subversion/include/svn_string.h
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_string.h?pathrev=35974&r1=35973&r2=35974
> >> ==============================================================================
> >> --- trunk/subversion/include/svn_string.h Thu Feb 19 02:17:08 2009 (r35973)
> >> +++ trunk/subversion/include/svn_string.h Thu Feb 19 03:41:36 2009 (r35974)
> >> @@ -192,7 +192,8 @@ svn_stringbuf_ncreate(const char *bytes,
> >> /** Create a new empty bytestring with at least @a minimum_size bytes of
> >> * space available in the memory block.
> >> *
> >> - * (@a minimum_size should include space for the terminating NULL character.)
> >> + * The allocated string buffer will be one byte larger then @a size to account
> >> + * for a final '\0'.
> >> *
> >> * @since New in 1.6.
> >> */
> >>
> >
> > ------------------------------------------------------
> > http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1191678
> >

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1191875
Received on 2009-02-19 14:41:41 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.