Re: [libvirt] [PATCHv6 2/5] virstring.h/c: Util method for making some find and replace in strings

2014-02-19 Thread Daniel P. Berrange
On Thu, Jan 23, 2014 at 10:28:30AM +0100, Manuel VIVES wrote:
 ---
  src/libvirt_private.syms |1 +
  src/util/virstring.c |   66 
 +-
  src/util/virstring.h |1 +
  3 files changed, 67 insertions(+), 1 deletion(-)

Can you also add a test case to src/virstringtest.c for this one
too. In particular test what happens when matches are exactly at
the start and end of the string  so we validate proper boundary
handling.

 +
 +/*
 + virStrReplace(haystack, oldneedle, newneedle) --
 +  Search haystack and replace all occurences of oldneedle with newneedle.
 +  Return a string with all the replacements in case of success, NULL in case
 +  of failure
 +*/
 +char *
 +virStrReplace(char *haystack,
 +  const char *oldneedle, const char *newneedle)

Can we call this  virStringReplace instead.

 +{
 +char *ret = NULL;
 +size_t i = strlen(haystack);
 +size_t count = 0;
 +size_t newneedle_len = strlen(newneedle);
 +size_t oldneedle_len = strlen(oldneedle);
 +int diff_len = newneedle_len - oldneedle_len;
 +size_t totalLength = 0;
 +size_t numberOfElementsToAllocate = 0;
 +if (!haystack || !oldneedle || !newneedle) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(null value for haystack, oldneedle or newneedle));
 +goto cleanup;
 +}
 +if (oldneedle_len == 0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(oldneedle cannot be empty));
 +goto cleanup;
 +}
 +if (diff_len == 0  memcmp(oldneedle, newneedle, newneedle_len) == 0) {
 +ignore_value(VIR_STRDUP(ret, haystack));
 +goto cleanup;
 +}
 +
 +for (i = 0; haystack[i] != '\0'; i++) {
 +if (memcmp(haystack[i], oldneedle, oldneedle_len) == 0) {

This looks like a out of bounds read. eg consider

   haystack = foo
   oldneedle = baar

You're going to be making memcmp read beyond the end of the haystack
array I believe.

 +count ++;
 +i += oldneedle_len - 1;
 +}
 +}
 +numberOfElementsToAllocate = (i + count * (newneedle_len - 
 oldneedle_len));
 +
 +if (VIR_ALLOC_N(ret, numberOfElementsToAllocate)  0)
 +goto cleanup;

Rather than trying to pre-caculate buffer lengths, I thin it would
probably be simpler if you just used the virBuffer APIs to construct
the new string, since that does grow-on-demand.
 +
 +totalLength = sizeof(char) * numberOfElementsToAllocate;
 +i = 0;
 +while (*haystack) {
 +if (memcmp(haystack, oldneedle, oldneedle_len) == 0) {
 +if (virStrcpy(ret[i], newneedle, totalLength) == NULL) {
 +VIR_FREE(ret);
 +goto cleanup;
 +}
 +totalLength -= newneedle_len;
 +i += newneedle_len;
 +haystack += oldneedle_len;
 +} else {
 +ret[i++] = *haystack++;
 +totalLength --;
 +}
 +}
 +ret[i] = '\0';
 +cleanup:
 +return ret;
 +}


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCHv6 2/5] virstring.h/c: Util method for making some find and replace in strings

2014-01-23 Thread Manuel VIVES
---
 src/libvirt_private.syms |1 +
 src/util/virstring.c |   66 +-
 src/util/virstring.h |1 +
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 68ca39d..5103919 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1778,6 +1778,7 @@ virStringSortRevCompare;
 virStringSplit;
 virStrncpy;
 virStrndup;
+virStrReplace;
 virStrToDouble;
 virStrToLong_i;
 virStrToLong_l;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 3c93450..b6f6192 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -618,7 +618,6 @@ size_t virStringListLength(char **strings)
 return i;
 }
 
-
 /**
  * virStringSortCompare:
  *
@@ -742,3 +741,68 @@ cleanup:
 VIR_FREE(pmatch);
 return ret;
 }
+
+/*
+ virStrReplace(haystack, oldneedle, newneedle) --
+  Search haystack and replace all occurences of oldneedle with newneedle.
+  Return a string with all the replacements in case of success, NULL in case
+  of failure
+*/
+char *
+virStrReplace(char *haystack,
+  const char *oldneedle, const char *newneedle)
+{
+char *ret = NULL;
+size_t i = strlen(haystack);
+size_t count = 0;
+size_t newneedle_len = strlen(newneedle);
+size_t oldneedle_len = strlen(oldneedle);
+int diff_len = newneedle_len - oldneedle_len;
+size_t totalLength = 0;
+size_t numberOfElementsToAllocate = 0;
+if (!haystack || !oldneedle || !newneedle) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(null value for haystack, oldneedle or newneedle));
+goto cleanup;
+}
+if (oldneedle_len == 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(oldneedle cannot be empty));
+goto cleanup;
+}
+if (diff_len == 0  memcmp(oldneedle, newneedle, newneedle_len) == 0) {
+ignore_value(VIR_STRDUP(ret, haystack));
+goto cleanup;
+}
+
+for (i = 0; haystack[i] != '\0'; i++) {
+if (memcmp(haystack[i], oldneedle, oldneedle_len) == 0) {
+count ++;
+i += oldneedle_len - 1;
+}
+}
+numberOfElementsToAllocate = (i + count * (newneedle_len - oldneedle_len));
+
+if (VIR_ALLOC_N(ret, numberOfElementsToAllocate)  0)
+goto cleanup;
+
+totalLength = sizeof(char) * numberOfElementsToAllocate;
+i = 0;
+while (*haystack) {
+if (memcmp(haystack, oldneedle, oldneedle_len) == 0) {
+if (virStrcpy(ret[i], newneedle, totalLength) == NULL) {
+VIR_FREE(ret);
+goto cleanup;
+}
+totalLength -= newneedle_len;
+i += newneedle_len;
+haystack += oldneedle_len;
+} else {
+ret[i++] = *haystack++;
+totalLength --;
+}
+}
+ret[i] = '\0';
+cleanup:
+return ret;
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index fe05403..2df0695 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -227,6 +227,7 @@ int virStringSortCompare(const void *a, const void *b);
 int virStringSortRevCompare(const void *a, const void *b);
 
 int virSearchRegex(const char *sourceString, unsigned int occurrence, const 
char *regexp, char **result);
+char * virStrReplace(char *haystack, const char *oldneedle, const char 
*newneedle);
 
 
 #endif /* __VIR_STRING_H__ */
-- 
1.7.10.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list