Re: [libvirt] [PATCHv4 1/5] viruuid.h/c: Util method for finding uuid patterns in some strings

> On 11/25/2013 07:23 PM, Manuel VIVES wrote: >> + * >> + * @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" > What is the use of having occurrence == -n, 0, or 1 yield the same > result? It makes more sense to define the function as "return occurrence > 'n' (starting from 0) of a sub-string that matches the pattern for a > uuid". Documenting is then much simpler, and it's easier to know the > "best" way to get the first occurrence (since there is only one way to > do it). > > This seems like a very specific use case for a more general function. > How about making a virSearchRegex() function (or maybe some other name) > which took the regex as another arg? That would make it more likely that > the code would be reused. > I have modified this method, so now it takes a regex as argument and I have renamed it to 'virSearchRegex'. Should I move it in another file (it is still in viruuid.c/h)?

>> + * @endcode >> + */ >> + >> +int >> +virSearchUuid(const char *sourceString, int occurrence, char **ret) >> +{ >> + unsigned 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; >> + int retCode = 0; >> + if (regcomp(&pregUuidBracket, uuidRegex, REG_EXTENDED) != 0) { > You're missing a blank line after the end of the data declarations. > Also, it's more common in libvirt to call the variable containing the > eventual function return value "ret" rather than "retCode", and > initialize it to -1. Then you just unconditionally set it to 0 just > before a successful return (or in this case, set it to 0 for the "not > found" case or 1 for the "found" case), but don't have to do anything to > it for all the failure cases. > I use retCode because I already have a parameter named ret which will contains the string matching, perhaps I should rename my parameter and use ret instead of retCode.