Well... "blocksize" is not something for users to worry about. It is
essentially an internal/private field. So I see no reason that the two
functions need to line up. Shoot... svn_stringbuf_ensure() could set
blocksize to almost anything!
Good point on the audit, but still... adding 1 seems much safer than
hoping callers will pass the right values. As we've already seen,
there is no obvious failure mode if they DO NOT. Why "hope" that
callers get it right, when we can just add a byte?
Cheers,
-g
On Thu, Feb 19, 2009 at 14:41, Chris Foote <cafoote_at_yahoo.com> wrote:
>> 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=1191929
Received on 2009-02-19 14:52:58 CET