On 04/25/2018 11:55 AM, Marc Hartmayer wrote:
1. Don't allocate if there is nothing that needs to be allocated. Especially as the result of calling calloc(0, ...) is implementation-defined.
Following VIR_ALLOC_N one finds : VIR_ALLOC_N(params_val, nparams) which equates to # define VIR_ALLOC_N(ptr, count) \ virAllocN(&(ptr), sizeof(*(ptr)), (count), true, \ VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__) or virAllocN(¶ms_val, sizeof(params_val), nparams, true, ...) and eventually/essentially *params_val = calloc(sizeof(params_val), nparams) If the returned value is NULL then we error w/ OOM (4th param=true). So, unless @params_val had no elements, it won't be calloc(0,...) and thus the question becomes is there a more specific path you are referencing here? FWIW: My f26 man page for calloc says: "The calloc() function allocates memory for an array of nmemb elements of size bytes each and returns a pointer to the allocated memory. The memory is set to zero. If nmemb or size is 0, then calloc() returns either NULL, or a unique pointer value that can later be successfully passed to free()" We have so many places in the code that use VIR_ALLOC_N and do not check if the sizeof() or count is 0 - which makes me wonder even more about the specific case in which you are referencing. I looked through the various remote_protocol.x structures that would use this and it seems they all use remote_typed_param for the structure being returned, so it's not clear how any of them could have a sizeof() returning 0.
2. Update the length @remote_params_len only if the related @remote_params_val has also been set.
This one changes the error case as the returned @remote_params_len changes from being set to @params_len on input to be undefined. John
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/util/virtypedparam.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 2452628cdbcd..10fd28baa65c 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -1500,10 +1500,10 @@ virTypedParamsSerialize(virTypedParameterPtr params, size_t i; size_t j; int rv = -1; - virTypedParameterRemotePtr params_val; + virTypedParameterRemotePtr params_val = NULL; + int params_len = nparams;
- *remote_params_len = nparams; - if (VIR_ALLOC_N(params_val, nparams) < 0) + if (nparams && VIR_ALLOC_N(params_val, nparams) < 0) goto cleanup;
for (i = 0, j = 0; i < nparams; ++i) { @@ -1515,7 +1515,7 @@ virTypedParamsSerialize(virTypedParameterPtr params, if (!param->type || (!(flags & VIR_TYPED_PARAM_STRING_OKAY) && param->type == VIR_TYPED_PARAM_STRING)) { - --*remote_params_len; + --params_len; continue; }
@@ -1556,6 +1556,7 @@ virTypedParamsSerialize(virTypedParameterPtr params, }
*remote_params_val = params_val; + *remote_params_len = params_len; params_val = NULL; rv = 0;