Use automatic freeing, declare one variable per line and return early when possible. As this is an internal helper there's no need to check that the caller passed non-NULL @values. Modify the documentation to be accurate and warn callers to not free the strings just the array. Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx> --- src/util/virtypedparam.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 634385aa97..86fbaf5e9d 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -432,13 +432,13 @@ virTypedParamsFilter(virTypedParameterPtr params, * @values: array of returned values * * Finds all parameters with desired @name within @params and - * store their values into @values. The @values array is self - * allocated and its length is stored into @picked. When no - * longer needed, caller should free the returned array, but not - * the items since they are taken from @params array. + * store their values into @values. * - * Returns amount of strings in @values array on success, - * -1 otherwise. + * Important: The strings in the returned string list @values are borrowed from + * @params and thus caller must free only the pointer returned as @values, but + * not the contents. + * + * Returns amount of strings in @values array on success. */ int virTypedParamsGetStringList(virTypedParameterPtr params, @@ -446,31 +446,26 @@ virTypedParamsGetStringList(virTypedParameterPtr params, const char *name, const char ***values) { - size_t i, n; + size_t i; + size_t n = 0; size_t nfiltered; - virTypedParameterPtr *filtered = NULL; + g_autofree virTypedParameterPtr *filtered = NULL; - virCheckNonNullArgGoto(values, error); *values = NULL; nfiltered = virTypedParamsFilter(params, nparams, name, &filtered); - if (nfiltered) - *values = g_new0(const char *, nfiltered); + if (nfiltered == 0) + return 0; + + *values = g_new0(const char *, nfiltered); - for (n = 0, i = 0; i < nfiltered; i++) { + for (i = 0; i < nfiltered; i++) { if (filtered[i]->type == VIR_TYPED_PARAM_STRING) (*values)[n++] = filtered[i]->value.s; } - VIR_FREE(filtered); return n; - - error: - if (values) - VIR_FREE(*values); - VIR_FREE(filtered); - return -1; } -- 2.46.0