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@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
> ---
> 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;
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
