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

> On 11/25/2013 07:23 PM, Manuel VIVES wrote: >> --- >> >> src/libvirt_private.syms | 1 + >> src/util/virstring.c | 48 >> ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virstring.h >> | 2 ++ >> 3 files changed, 51 insertions(+) >> >> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >> index 99cc32a..b761fb6 100644 >> --- a/src/libvirt_private.syms >> +++ b/src/libvirt_private.syms >> @@ -1750,6 +1750,7 @@ virStringListLength; >> >> virStringSplit; >> virStrncpy; >> virStrndup; >> >> +virStrReplace; >> >> virStrToDouble; >> virStrToLong_i; >> virStrToLong_l; >> >> diff --git a/src/util/virstring.c b/src/util/virstring.c >> index d11db5c..a30a4ef 100644 >> --- a/src/util/virstring.c >> +++ b/src/util/virstring.c >> @@ -616,3 +616,51 @@ size_t virStringListLength(char **strings) >> >> return i; >> >> } >> >> + >> +/* >> + 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; >> + size_t i, count = 0; >> + size_t newneedle_len = strlen(newneedle); >> + size_t oldneedle_len = strlen(oldneedle); >> + size_t totalLength = 0; >> + if (strlen(oldneedle) == 0) { > You can just check oldneedle_len here since it already contains > strlen(oldneedle). > > Also, should this really be a NOP? It makes no sense to ask to replace > all occurrences of "" with anything (kind of like dividing by 0), so I > think it should rather be an error (likewise, do we need to check for > !oldneedle or !newneedle ? Or are the callers controlled enough that we > can just put that requirement on them? If the latter, then the > declaration in the .h file needs to have "ATTRIBUTE_NONNULL(2), > ATTRIBUTE_NONNULL(3) (and we probably should have ATTRIBUTE_NONNULL(1) > in any case, unless there is a danger a caller could send a null haystack) > I'm going to modify my patch in order to return an error if oldneedle is empty. I also looked at ATTRIBUTE_NONNULL and I did't really understand the purpose so if someone could explain it ;)

But maybe I should also include the !oldneedle, the !newneedle and !haystack tests inside the method, what do you think?