kfogel@collab.net wrote:
> Martin Furter <mf@rola.ch> writes:
>
>>I'm not sure if it's supposed to be this way but
>>svn_string_strip_whitespace does not strip any spaces when the string
>>only contains spaces.
...
>>Is this a bug ?
>
> I think it's a bug. The doc string implies that it should reduce such
> a string to 0 length. Want to fix?
How about this? This patch fixes this bug and the other one that I found, but gives the ...string... and ...stringbuf... versions different interfaces. If you agree that's reasonable, I'll commit.
One alternative is to delete svn_string_strip_whitespace, as nothing within Subversion uses it yet.
Another is to convert svn_stringbuf_strip_whitespace also to use a copy-and-return interface.
- Julian
[[[
Fix whitespace-trimming functions: they did not trim a string which consisted
only of whitespace, and one of them violated its invariants.
* subversion/include/svn_string.h
Add a note to the module documentation.
(svn_string_strip_whitespace): Can't modify in place, so return a new string.
* subversion/libsvn_subr/svn_string.c
(svn_string_strip_whitespace): Can't modify in place, so return a new string.
Strip even if string is all whitespace.
(svn_stringbuf_strip_whitespace): Strip even if string is all whitespace.
]]]
Index: subversion/include/svn_string.h
===================================================================
--- subversion/include/svn_string.h (revision 7362)
+++ subversion/include/svn_string.h (working copy)
@@ -33,6 +33,8 @@
* @c svn_stringbuf_t uses a plain <tt>char *</tt> for its data, so it is
* most appropriate for modifiable data.
*
+ * A function that returns (a pointer to) a string always returns a deep copy.
+ *
* <h3>Invariants</h3>
*
* 1. Null termination:
@@ -118,7 +120,7 @@
apr_pool_t *pool);
/** Create a new bytestring containing a generic string of bytes
- * (NON-null-terminated) */
+ * (NOT null-terminated) */
svn_string_t *svn_string_ncreate (const char *bytes,
const apr_size_t size,
apr_pool_t *pool);
@@ -159,8 +161,9 @@
*/
apr_size_t svn_string_first_non_whitespace (const svn_string_t *str);
-/** Strips whitespace from both sides of @a str (modified in place). */
-void svn_string_strip_whitespace (svn_string_t *str);
+/** Strip whitespace from both sides of @a str. */
+svn_string_t *svn_string_strip_whitespace (const svn_string_t *str,
+ apr_pool_t *pool);
/** Return position of last occurrence of @a char in @a str, or return
* @a str->len if no occurrence.
@@ -265,7 +268,7 @@
*/
apr_size_t svn_stringbuf_first_non_whitespace (const svn_stringbuf_t *str);
-/** Strips whitespace from both sides of @a str (modified in place). */
+/** Strip whitespace from both sides of @a str (modified in place). */
void svn_stringbuf_strip_whitespace (svn_stringbuf_t *str);
/** Return position of last occurrence of @a ch in @a str, or return
Index: subversion/libsvn_subr/svn_string.c
===================================================================
--- subversion/libsvn_subr/svn_string.c (revision 7362)
+++ subversion/libsvn_subr/svn_string.c (working copy)
@@ -179,41 +179,20 @@
}
-void
-svn_string_strip_whitespace (svn_string_t *str)
+svn_string_t *
+svn_string_strip_whitespace (const svn_string_t *str, apr_pool_t *pool)
{
- apr_size_t i;
-
/* Find first non-whitespace character */
apr_size_t offset = svn_string_first_non_whitespace (str);
-
- if (offset == str->len)
- return;
-
- /* Go ahead! Waste some RAM, we've got pools! :) */
- str->data += offset;
- str->len -= offset;
- /* Can't adjust `blocksize' here, because svn_string_t does not
- record a blocksize independent of len, thus can't be
- realloc'd. */
+ const char *data = str->data + offset;
+ apr_size_t len = str->len - offset;
/* Now that we've chomped whitespace off the front, search backwards
from the end for the first non-whitespace. */
+ while ((len > 0) && apr_isspace (data[len - 1]))
+ len--;
- for (i = (str->len - 1); i >= 0; i--)
- {
- if (! apr_isspace (str->data[i]))
- {
- break;
- }
- }
-
- /* Mmm, waste some more RAM */
- str->len = i + 1;
-
- /* ### In svn_stringbuf_strip_whitespace, we reset the null
- terminator here. But svn_string_t can have const data, so I
- don't think we can do that, unfortunately. */
+ return svn_string_ncreate (data, len, pool);
}
@@ -462,32 +441,17 @@
void
svn_stringbuf_strip_whitespace (svn_stringbuf_t *str)
{
- apr_size_t i;
-
/* Find first non-whitespace character */
apr_size_t offset = svn_stringbuf_first_non_whitespace (str);
- if (offset == str->len)
- return;
-
/* Go ahead! Waste some RAM, we've got pools! :) */
str->data += offset;
str->len -= offset;
str->blocksize -= offset;
- /* Now that we've chomped whitespace off the front, search backwards
- from the end for the first non-whitespace. */
-
- for (i = (str->len - 1); i >= 0; i--)
- {
- if (! apr_isspace (str->data[i]))
- {
- break;
- }
- }
-
- /* Mmm, waste some more RAM */
- str->len = i + 1;
+ /* Now that we've trimmed the front, trim the end, wasting more RAM. */
+ while ((str->len > 0) && apr_isspace (str->data[str->len - 1]))
+ str->len--;
str->data[str->len] = '\0';
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Oct 9 01:50:06 2003