On 12.05.2015 14:07, Pavel Boldin wrote: > Add multikey APIs for virTypedParams*: > > * virTypedParamsPick that returns all the parameters with the > specified name and type. > * virTypedParamsPickStrings that returns a NULL-terminated `const char**' > list with all the values for specified name and string type. > > Signed-off-by: Pavel Boldin <pboldin@xxxxxxxxxxxx> > --- > include/libvirt/libvirt-host.h | 10 +++++ > src/libvirt_public.syms | 6 +++ > src/util/virtypedparam.c | 90 ++++++++++++++++++++++++++++++++++++++++++ > tests/virtypedparamtest.c | 87 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 193 insertions(+) > > diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h > index a3e8b88..afa730f 100644 > --- a/include/libvirt/libvirt-host.h > +++ b/include/libvirt/libvirt-host.h > @@ -256,6 +256,12 @@ virTypedParameterPtr > virTypedParamsGet (virTypedParameterPtr params, > int nparams, > const char *name); > +virTypedParameterPtr* s/Ptr*/Ptr */ > +virTypedParamsPick (virTypedParameterPtr params, > + int nparams, > + const char *name, > + int type, > + size_t *picked); > int > virTypedParamsGetInt (virTypedParameterPtr params, > int nparams, > @@ -291,6 +297,10 @@ virTypedParamsGetString (virTypedParameterPtr params, > int nparams, > const char *name, > const char **value); > +const char ** > +virTypedParamsPickStrings(virTypedParameterPtr params, > + int nparams, > + const char *name); > int > virTypedParamsAddInt (virTypedParameterPtr *params, > int *nparams, > diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms > index ef3d2f0..d886967 100644 > --- a/src/libvirt_public.syms > +++ b/src/libvirt_public.syms > @@ -710,4 +710,10 @@ LIBVIRT_1.2.15 { > virDomainDelIOThread; > } LIBVIRT_1.2.14; > > +LIBVIRT_1.2.16 { > + global: > + virTypedParamsPick; > + virTypedParamsPickStrings; > +} LIBVIRT_1.2.15; > + > # .... define new API here using predicted next version number .... > diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c > index bd47609..643d10e 100644 > --- a/src/util/virtypedparam.c > +++ b/src/util/virtypedparam.c > @@ -480,6 +480,56 @@ virTypedParamsGet(virTypedParameterPtr params, > } > > > +/** > + * virTypedParamsPick: > + * @params: array of typed parameters > + * @nparams: number of parameters in the @params array > + * @name: name of the parameter to find > + * @type: type of the parameter to find > + * @picked: pointer to a size_t with amount of picked Maybe "the length of returned array"? > + * > + * > + * Finds typed parameters called @name. > + * > + * Returns pointers to the parameters or NULL if there are none in @params. > + * This function does not raise an error, even when returning NULL. Generally, it's not okay for a public error to not throw an error. Returning NULL is a valid case though. However I think we need to distinguish between no result returned and an error occurring .. [1] > + * Callee should call VIR_FREE on the returned array. Don't want to be picky, but s/Callee/Caller/ and /call VIR_FREE on/free/. And maybe mention that it's only the array that needs to be freed - items within it are just copied. > + */ > +virTypedParameterPtr* > +virTypedParamsPick(virTypedParameterPtr params, > + int nparams, > + const char *name, > + int type, > + size_t *picked) > +{ > + size_t i, max = 0; > + virTypedParameterPtr *values = NULL; > + This is a public API. The very first thing it has to do is call virResetLastError(). > + *picked = 0; Nah, this should be checked for NULL. > + > + if (!params || !name) > + return NULL; > + > + for (i = 0; i < nparams; i++) { > + if (params[i].type == type && STREQ(params[i].field, name)) { > + if (VIR_RESIZE_N(values, max, *picked, 1) < 0) 1: .. as a result of OOM. So maybe the function header should look more like this: int virTypedParamsFilter(virTypedParameterPtr params, int nparams, const char *name, int type, virTypedParameterPtr *ret, size_t *picked); Also, is the @type needed? I guess we can go with just @name. Picking all the params that have desired key name. > + goto error; > + > + values[*picked] = ¶ms[i]; > + > + *picked += 1; > + } > + } > + > + return values; > + > + error: > + *picked = 0; > + VIR_FREE(values); > + return NULL; > +} > + > + > #define VIR_TYPED_PARAM_CHECK_TYPE(check_type) \ > do { if (param->type != check_type) { \ > virReportError(VIR_ERR_INVALID_ARG, \ > @@ -747,6 +797,46 @@ virTypedParamsGetString(virTypedParameterPtr params, > } > > > +/** > + * virTypedParamsPickStrings: > + * @params: array of typed parameters > + * @nparams: number of parameters in the @params array > + * @name: name of the parameter to find > + * > + * > + * Finds typed parameters called @name. > + * > + * Returns pointers to the parameters or NULL if there are none in @params. Returns a string list, which is a NULL terminated array of pointers to strings. > + * This function does not raise an error, even when returning NULL. > + * Callee should call VIR_FREE on the returned array. > + */ > +const char ** > +virTypedParamsPickStrings(virTypedParameterPtr params, > + int nparams, const char *name) > +{ > + const char **values = NULL; > + size_t i, picked; > + virTypedParameterPtr *picked_params; Again, public API, virResetLastError() should go here. > + > + picked_params = virTypedParamsPick(params, nparams, > + name, VIR_TYPED_PARAM_STRING, > + &picked); > + > + if (picked_params == NULL) > + return NULL; > + > + if (VIR_ALLOC_N(values, picked + 1) < 0) > + goto cleanup; > + > + for (i = 0; i < picked; i++) > + values[i] = picked_params[i]->value.s; Hm.. Currently we allow virTypedParamsAddString(params, &nparams, &maxparams, "some key name", NULL); where NULL is the value. Not that it would be used anywhere, but we allow it. It would break the list here. So maybe we need to return the length of the list too? > + > + cleanup: > + VIR_FREE(picked_params); > + return values; > +} This function suffers the same problems as the previous one. I guess we need slightly different function header. > + > + > #define VIR_TYPED_PARAM_CHECK() \ > do { if (virTypedParamsGet(*params, n, name)) { \ > virReportError(VIR_ERR_INVALID_ARG, \ See? This is the check I'm mentioning in 1/5. > diff --git a/tests/virtypedparamtest.c b/tests/virtypedparamtest.c > index 27b7e60..287d3f1 100644 > --- a/tests/virtypedparamtest.c > +++ b/tests/virtypedparamtest.c > @@ -95,6 +95,87 @@ testTypedParamsValidate(const void *opaque) > #define PARAM(field_, type_) { .field = field_, .type = type_ } > > static int > +testTypedParamsPick(const void *opaque ATTRIBUTE_UNUSED) > +{ > + size_t i, picked; > + int rv = -1; > + virTypedParameter params[] = { > + { .field = "bar", .type = VIR_TYPED_PARAM_UINT }, > + { .field = "foo", .type = VIR_TYPED_PARAM_INT }, > + { .field = "bar", .type = VIR_TYPED_PARAM_UINT }, > + { .field = "foo", .type = VIR_TYPED_PARAM_INT }, > + { .field = "foobar", .type = VIR_TYPED_PARAM_STRING }, > + { .field = "foo", .type = VIR_TYPED_PARAM_INT } > + }; > + virTypedParameterPtr *picked_params = NULL; > + > + > + picked_params = virTypedParamsPick(params, ARRAY_CARDINALITY(params), > + "foo", VIR_TYPED_PARAM_INT, &picked); > + if (picked != 3) > + goto cleanup; > + > + for (i = 0; i < picked; i++) { > + if (picked_params[i] != ¶ms[1 + i * 2]) > + goto cleanup; > + } > + VIR_FREE(picked_params); > + > + picked_params = virTypedParamsPick(params, ARRAY_CARDINALITY(params), > + "bar", VIR_TYPED_PARAM_UINT, &picked); > + > + if (picked != 2) > + goto cleanup; > + > + for (i = 0; i < picked; i++) { > + if (picked_params[i] != ¶ms[i * 2]) > + goto cleanup; > + } > + > + rv = 0; > + cleanup: > + VIR_FREE(picked_params); > + return rv; > +} > + > +static int > +testTypedParamsPickStrings(const void *opaque ATTRIBUTE_UNUSED) > +{ > + size_t i; > + int rv = -1; > + const char **strings = NULL; > + > + virTypedParameter params[] = { > + { .field = "bar", .type = VIR_TYPED_PARAM_STRING, > + .value = { .s = (char*)"bar1"} }, > + { .field = "foo", .type = VIR_TYPED_PARAM_INT }, > + { .field = "bar", .type = VIR_TYPED_PARAM_STRING, > + .value = { .s = (char*)"bar2"} }, > + { .field = "foo", .type = VIR_TYPED_PARAM_INT }, > + { .field = "foobar", .type = VIR_TYPED_PARAM_STRING }, > + { .field = "foo", .type = VIR_TYPED_PARAM_INT }, > + { .field = "bar", .type = VIR_TYPED_PARAM_STRING, > + .value = { .s = (char*)"bar3"} } > + }; > + > + strings = virTypedParamsPickStrings(params, > + ARRAY_CARDINALITY(params), > + "bar"); > + > + for (i = 0; strings[i]; i++) { > + if (!STREQLEN(strings[i], "bar", 3)) > + goto cleanup; > + if (strings[i][3] != i + '1') > + goto cleanup; > + } > + > + rv = 0; > + cleanup: > + VIR_FREE(strings); > + return rv; > +} > + > +static int > testTypedParamsValidator(void) > { > size_t i; > @@ -169,6 +250,12 @@ mymain(void) > if (testTypedParamsValidator() < 0) > rv = -1; > > + if (virtTestRun("Picking", testTypedParamsPick, NULL) < 0) > + rv = -1; > + > + if (virtTestRun("Picking Strings", testTypedParamsPickStrings, NULL) < 0) > + rv = -1; > + > if (rv < 0) > return EXIT_FAILURE; > return EXIT_SUCCESS; > Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list