On 09/02/2013 06:05 PM, Manuel VIVES wrote:
--- src/libvirt_private.syms | 1 + src/util/viruuid.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/viruuid.h | 1 + 3 files changed, 81 insertions(+)
@@ -333,3 +337,78 @@ int virGetHostUUID(unsigned char *uuid)
return ret; } + + +/** + * virSearchUuid: + * Return the nth occurrence of a substring in sourceString which matches an uuid pattern + * If there is no substring, ret is not modified + * + * @sourceString: String to parse + * @occurrence: We will return the nth occurrence of uuid in substring, if equals to 0 (or negative), will return the first occurence + * @ret: nth occurrence substring matching an uuid pattern + * @code + char *source = "6853a496-1c10-472e-867a-8244937bd6f0 773ab075-4cd7-4fc2-8b6e-21c84e9cb391 bbb3c75c-d60f-43b0-b802-fd56b84a4222 60c04aa1-0375-4654-8d9f-e149d9885273 4548d465-9891-4c34-a184-3b1c34a26aa8"; + char *ret1=NULL; + char *ret2=NULL; + char *ret3=NULL; + char *ret4=NULL; + virSearchUuid(source, 4,&ret1); //ret1 = "60c04aa1-0375-4654-8d9f-e149d9885273" + virSearchUuid(source, 0,&ret2); //ret2 = "6853a496-1c10-472e-867a-8244937bd6f0" + virSearchUuid(source, 1,&ret3); //ret3 = "6853a496-1c10-472e-867a-8244937bd6f0" + virSearchUuid(source, -4,&ret4); //ret4 = "6853a496-1c10-472e-867a-8244937bd6f0" + * @endcode + */ + +char ** +virSearchUuid(const char *sourceString, int occurrence,char **ret)
char ** func(..., char **ret) seems redundant. Either drop the 'ret' parameter completely and return just char, or change the return value to int (e.g. -1 = error, 0 = not found, 1 = found).
+{ + int position = ((occurrence -1) > 0) ? (occurrence -1) : 0; + + const char *uuidRegex = "([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})"; + regex_t pregUuidBracket; + size_t i = 0; + size_t nmatch = 0; + regmatch_t *pmatch = NULL; + if (regcomp(&pregUuidBracket, uuidRegex, REG_EXTENDED) != 0) { + VIR_DEBUG("Error while compiling regular expression");
This should be virReportError.
+ goto cleanup;
and a return here, or you'll call regfree on an uninitialized variable.
+ } + nmatch = pregUuidBracket.re_nsub; + if (VIR_ALLOC_N(pmatch, nmatch) != 0) { + virReportOOMError();
VIR_ALLOC already reports an error.
+ goto cleanup; + } + while (i < (position+1)) {
You should exit the cycle on the first regexec failure, there's no point in calling it again and again with the same arguments.
+ if (regexec(&pregUuidBracket, sourceString, nmatch, pmatch, 0) == 0) {
All this
+ char *substring = NULL; + int start = pmatch[0].rm_so; + int end = pmatch[0].rm_eo; + size_t size = end - start; + if (VIR_ALLOC_N(substring, (size + 1)) != 0) { + virReportOOMError(); + goto cleanup; + } + if (substring) { + if (virStrncpy(substring, &sourceString[start], size, size + 1)) { + substring[size] = '\0'; + if (VIR_STRDUP(*ret, substring) < 0) { + VIR_DEBUG("cannot duplicate %s", substring); + goto cleanup; + } + } + VIR_FREE(substring); + }
can be replaced by: regoff_t start = pmatch[0].rm_so; regoff_t end = pmatch[0].rm_eo; if (VIR_STRNDUP(*ret, sourceString + start, end - start) < 0) goto cleanup; But even then, by copying every UUID to *ret, you'd leak the string buffer allocated for every one except the last one.
+ sourceString = &sourceString[end]; + } + ++i; + }
These three lines are redundant:
+ regfree(&pregUuidBracket); + VIR_FREE(pmatch); + return ret;
+ +cleanup: + regfree(&pregUuidBracket); + VIR_FREE(pmatch); + return ret; +}
And I'm sorry, but I have no opinion on the rest of the patches. Jan