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 = "baaaaaar" 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 :|