Re: [PATCH 1/2] virTypedParamsSerialize: minor fixes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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(&params_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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux