Rev svn_stringbuf_ensure() to provide a guarantee that an additional byte will be allocated to hold the null terminator, thus bringing it in line with the semantics of svn_stringbuf_create_ensure(). Some callers had been expecting this behaviour, and thus had been buggy. * subversion/include/svn_string.h (svn_stringbuf_ensure2): New function. (svn_stringbuf_ensure): Deprecate. * subversion/libsvn_subr/svn_string.c (svn_stringbuf_ensure2): New function, exactly as svn_stringbuf_ensure() except adding one to the requested length parameter. (svn_stringbuf_ensure): Move from here ... * ../subversion/libsvn_subr/deprecated.c (svn_stringbuf_ensure): ... to here, and re-write as a simple call to svn_stringbuf_ensure2(), with a comment explaining why. --This line, and those below, will be ignored-- Fix up issues with the new svn_stringbuf_create_empty() function. Unlike svn_string_create_empty(), the resulting string buffer is likely to have data appended to it. The previous implementation created a non-standard buffer that claimed that no space was allocated, not even for the null terminator, which was strictly incorrect and meant it would always need reallocation before appending any data. By calling the standard create function, we ensure the class invariants are maintained without any tricks, and also ensure there is enough space for a little expansion before reallocation is required. * subversion/include/svn_string.h (svn_stringbuf_create_empty): Don't claim that the allocated size will be zero. * subversion/libsvn_subr/svn_string.c (empty_buffer): Add a doc string. ### (create_stringbuf): Add a doc string and assertions. (svn_stringbuf_create_empty): Call svn_stringbuf_ncreate() instead of specially constructing an empty stringbuf. --This line, and those below, will be ignored-- * subversion/libsvn_subr/svn_string.c (svn_stringbuf__morph_into_string): Fix the debug code to specify the correct allocated size. This bug could have resulted in spurious failures in debug mode. --This line, and those below, will be ignored-- * subversion/include/svn_string.h (svn_string_create, svn_string_create_empty, svn_string_ncreate, svn_string_create_from_buf, svn_string_createf, svn_string_createv, svn_stringbuf_create, svn_stringbuf_ncreate, svn_stringbuf_create_empty, svn_stringbuf_create_ensure, svn_stringbuf_create_from_string, svn_stringbuf_createf, svn_stringbuf_createv): Rewrite doc strings to make clear that input data is copied, describe all (except pool) parameters by name, and use consistent wording. --This line, and those below, will be ignored-- Index: ../subversion/include/svn_string.h =================================================================== --- ../subversion/include/svn_string.h (revision 1243880) +++ ../subversion/include/svn_string.h (working copy) @@ -125,34 +125,41 @@ typedef struct svn_stringbuf_t * @{ */ -/** Create a new bytestring containing a C string (NULL-terminated). */ +/** Create a new string copied from a (NULL-terminated) C string, + * @a cstring. + */ svn_string_t * svn_string_create(const char *cstring, apr_pool_t *pool); -/** Create a truely empty string object (length is 0) +/** Create a new, empty string (length is zero). + * * @since New in 1.8. */ svn_string_t * svn_string_create_empty(apr_pool_t *pool); -/** Create a new bytestring containing a generic string of bytes - * (NOT NULL-terminated) */ +/** Create a new string copied from a generic string of bytes, @a bytes, of + * length @a size bytes. @a bytes is NOT assumed to be NULL-terminated, but + * the new string will be. + */ svn_string_t * svn_string_ncreate(const char *bytes, apr_size_t size, apr_pool_t *pool); -/** Create a new string with the contents of the given stringbuf */ +/** Create a new string copied from the stringbuf @a strbuf. + */ svn_string_t * svn_string_create_from_buf(const svn_stringbuf_t *strbuf, apr_pool_t *pool); -/** Create a new bytestring by formatting @a cstring (NULL-terminated) - * from varargs, which are as appropriate for apr_psprintf(). +/** Create a new string by printf-style formatting using @a fmt and the + * variable arguments, which are as appropriate for apr_psprintf(). */ svn_string_t * svn_string_createf(apr_pool_t *pool, const char *fmt, ...) __attribute__((format(printf, 2, 3))); -/** Create a new bytestring by formatting @a cstring (NULL-terminated) - * from a @c va_list (see svn_stringbuf_createf()). +/** Create a new string by printf-style formatting using @c fmt and @a ap. + * This is the same as svn_string_createf() except for the different + * way of passing the variable arguments. */ svn_string_t * svn_string_createv(apr_pool_t *pool, const char *fmt, va_list ap) @@ -190,56 +197,72 @@ svn_string_find_char_backward(const svn_ * @{ */ -/** Create a new bytestring containing a C string (NULL-terminated). */ +/** Create a new string copied from a (NULL-terminated) C string, + * @a cstring. + */ svn_stringbuf_t * svn_stringbuf_create(const char *cstring, apr_pool_t *pool); -/** Create a new bytestring containing a generic string of bytes - * (NON-NULL-terminated) +/** Create a new string copied from a generic string of bytes, @a bytes, of + * length @a size bytes. @a bytes is NOT assumed to be NULL-terminated, but + * the new string will be. */ svn_stringbuf_t * svn_stringbuf_ncreate(const char *bytes, apr_size_t size, apr_pool_t *pool); -/** Create a truely empty string object (length and blocksize are 0) +/** Create a new, empty string (length is zero). + * * @since New in 1.8. */ svn_stringbuf_t * svn_stringbuf_create_empty(apr_pool_t *pool); -/** Create a new empty bytestring with at least @a minimum_size bytes of - * space available in the memory block. - * - * The allocated string buffer will be one byte larger than @a minimum_size - * to account for a final '\\0'. +/** Create a new empty bytestring with at least enough space allocated to + * store a string of @a minimum_size bytes in length, and enough space for + * the terminating NULL character as well. * * @since New in 1.6. */ svn_stringbuf_t * svn_stringbuf_create_ensure(apr_size_t minimum_size, apr_pool_t *pool); -/** Create a new stringbuf with the contents of the given string */ +/** Create a new string copied from the string @a str. + */ svn_stringbuf_t * svn_stringbuf_create_from_string(const svn_string_t *str, apr_pool_t *pool); -/** Create a new bytestring by formatting @a cstring (NULL-terminated) - * from varargs, which are as appropriate for apr_psprintf(). +/** Create a new string by printf-style formatting using @a fmt and the + * variable arguments, which are as appropriate for apr_psprintf(). */ svn_stringbuf_t * svn_stringbuf_createf(apr_pool_t *pool, const char *fmt, ...) __attribute__((format(printf, 2, 3))); -/** Create a new bytestring by formatting @a cstring (NULL-terminated) - * from a @c va_list (see svn_stringbuf_createf()). +/** Create a new string by printf-style formatting using @c fmt and @a ap. + * This is the same as svn_stringbuf_createf() except for the different + * way of passing the variable arguments. */ svn_stringbuf_t * svn_stringbuf_createv(apr_pool_t *pool, const char *fmt, va_list ap) __attribute__((format(printf, 2, 0))); +/** Make sure that @a str has at least enough space allocated to store a + * string of @a minimum_size bytes in length, and enough space for the + * terminating NULL character as well. + * + * @since New in 1.8. + */ +void +svn_stringbuf_ensure2(svn_stringbuf_t *str, apr_size_t minimum_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.) + * + * @deprecated Provided for backward compatibility with the 1.7 API. */ +SVN_DEPRECATED void svn_stringbuf_ensure(svn_stringbuf_t *str, apr_size_t minimum_size); Index: ../subversion/libsvn_subr/deprecated.c =================================================================== --- ../subversion/libsvn_subr/deprecated.c (revision 1243880) +++ ../subversion/libsvn_subr/deprecated.c (working copy) @@ -1199,3 +1199,18 @@ svn_utf_initialize(apr_pool_t *pool) { svn_utf_initialize2(pool, FALSE); } + + +/*** From subst.c ***/ + +void +svn_stringbuf_ensure(svn_stringbuf_t *str, apr_size_t minimum_size) +{ + /* Provide at least one byte more than requested, because it is easy for + * callers to mistake the 'minimum_size' parameter of this API with the + * 'minimum_size' parameter of svn_stringbuf_create_ensure() which is + * defined differently (it doesn't include the terminating NUL). If it + * were not for this consideration, we would pass 'minimum_size - 1' to + * svn_stringbuf_ensure2(). */ + svn_stringbuf_ensure2(str, minimum_size); +} Index: ../subversion/libsvn_subr/svn_string.c =================================================================== --- ../subversion/libsvn_subr/svn_string.c (revision 1243880) +++ ../subversion/libsvn_subr/svn_string.c (working copy) @@ -132,8 +132,14 @@ create_string(const char *data, apr_size return new_string; } +/* A data buffer for a zero-length string (just a null terminator). Many + * svn_string_t instances may share this same buffer. + * + * ### TODO: Just share the ncreate(0) code, instead of introducing this + * different code path. + * ### It is writable because ...? */ static char empty_buffer[1] = {0}; - + svn_string_t * svn_string_create_empty(apr_pool_t *pool) { @@ -255,7 +261,7 @@ svn_stringbuf__morph_into_string(svn_str */ #ifdef SVN_DEBUG strbuf->pool = NULL; - strbuf->blocksize = strbuf->len; + strbuf->blocksize = strbuf->len + 1; #endif /* Both, svn_string_t and svn_stringbuf_t are public API structures @@ -281,6 +287,9 @@ svn_stringbuf__morph_into_string(svn_str /* svn_stringbuf functions */ +/* Create a stringbuf referring to (not copying) an existing block of memory + * at DATA, of which SIZE bytes are the user data and BLOCKSIZE bytes are + * allocated in total. DATA[SIZE] must be a zero byte. */ static svn_stringbuf_t * create_stringbuf(char *data, apr_size_t size, apr_size_t blocksize, apr_pool_t *pool) @@ -289,6 +298,9 @@ create_stringbuf(char *data, apr_size_t new_string = apr_palloc(pool, sizeof(*new_string)); + SVN_ERR_ASSERT_NO_RETURN(size < blocksize); + SVN_ERR_ASSERT_NO_RETURN(data[size] == '\0'); + new_string->data = data; new_string->len = size; new_string->blocksize = blocksize; @@ -300,12 +312,7 @@ create_stringbuf(char *data, apr_size_t svn_stringbuf_t * svn_stringbuf_create_empty(apr_pool_t *pool) { - /* All instances share the same zero-length buffer. - * Some algorithms, however, assume that they may write - * the terminating zero. So, empty_buffer must be writable - * (a simple (char *)"" will cause SEGFAULTs). */ - - return create_stringbuf(empty_buffer, 0, 0, pool); + return svn_stringbuf_create_ensure(0, pool); } svn_stringbuf_t * @@ -439,8 +446,10 @@ svn_stringbuf_isempty(const svn_stringbu void -svn_stringbuf_ensure(svn_stringbuf_t *str, apr_size_t minimum_size) +svn_stringbuf_ensure2(svn_stringbuf_t *str, apr_size_t minimum_size) { + ++minimum_size; /* + space for '\0' */ + /* Keep doubling capacity until have enough. */ if (str->blocksize < minimum_size) {