On Thu, Jun 21, 2018 at 03:47 PM +0200, Marc Hartmayer <mhartmay@xxxxxxxxxxxxx> wrote: > On Wed, Jun 13, 2018 at 03:11 PM +0200, John Ferlan <jferlan@xxxxxxxxxx> wrote: >> On 06/07/2018 08:17 AM, Marc Hartmayer wrote: >>> On Wed, May 09, 2018 at 09:51 PM +0200, John Ferlan <jferlan@xxxxxxxxxx> wrote: >>>> On 05/07/2018 11:24 AM, Marc Hartmayer wrote: >>>>> On Mon, Apr 30, 2018 at 03:20 PM +0200, John Ferlan <jferlan@xxxxxxxxxx> 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. >>>> >>>> uh oh - another memory recollection challenge ;-) >>>> >>>>>> >>>>>> 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 >>>>> >>>>> I'm talking about the case that nparams == 0 => calloc(sizeof(…), 0). >>>>> >>>> >>>> And I was thinking is there a specific consumer/caller of >>>> virTypedParamsSerialize that was passing something incorrectly. >>>> >>>>>> thus the question becomes is there a more specific path you are >>>>>> referencing here? >>>>> >>>>> It’s a “generic” serializer so it should work for every (valid) >>>>> case. What happens, for example, if params == NULL and nparams == 0? In >>>>> that case I would expect *remote_params_val = NULL and >>>>> *remote_params_len = 0. >>>>> >>>> >>>> Going through the callers to virTypedParamsSerialize can/does anyone >>>> pass params == NULL and nparams == 0? Would that be reasonable and/or >>>> expected? >>>> >>>> My look didn't find any - either some caller checks that the passed >>>> @params != NULL (usually via virCheckNonNullArgGoto in the external API >>>> call) or @params is built up on the fly and wouldn't be NULL because >>>> there'd be an error causing failure beforehand. Although I'll admit the >>>> migration ones are also crazy to look at. >>>> >>>> I could have made a mistake too; hence, the is there a specific instance >>>> that I missed? Or perhaps this is a result of some branched/private code >>>> that had that error which I don't have access to? >>> >>> It was the result of private code but actually it was intended and no >>> error :) >>> >>>> >>>> To answer your "What happens, for example, if params == NULL and nparams >>>> == 0?", well supposedly "VIR_ALLOC_N(params_val, 0)" should return NULL, >>>> so params_val and thus *remote_params_val == NULL. >>> >>> Did you try what 'VIR_ALLOC_N(params_val, 0)' actually returns? At least >>> for me, it doesn’t return a NULL pointer. >>> >> >> I think I already determined that NULL isn't returned; otherwise, we >> would have failed w/ OOM. I do recall trying this and seeing a non NULL >> value returned. >> >> IIRC: I found no way into [de]serialize w/ a 0 parameter, so having that >> guard didn't seem necessary, > > I used the libvirt-python APIs for some testing. I called an (own) API > with 'None' as argument and this resulted in 0 parameters. Maybe it was an empty dictionary. […snip] -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
