[prev in list] [next in list] [prev in thread] [next in thread]
List: libvir-list
Subject: Re: [libvirt] [PATCH 1/2] virTypedParamsSerialize: minor fixes
From: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= <berrange () redhat ! com>
Date: 2018-04-30 13:28:00
Message-ID: 20180430132800.GK3249 () redhat ! com
[Download RAW message or body]
On Mon, Apr 30, 2018 at 09:20:29AM -0400, John Ferlan wrote:
>
>
> 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.
If there is any problem with this, then we should just change bootstrap.conf
to use calloc-gnu instead of calloc-posix, which basically turns calloc(0)
into calloc(1) for compat with glibc behaviour.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic