Re: [libvirt] [PATCHv2 RESEND 1/5] viruuid.h/c: Util method for finding uuid patterns in some strings
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