On 09/13/14 15:27, John Ferlan wrote: > Since 98b9acf5aa02551dd37d0209339aba2e22e4004a > > This was a false positive where Coverity was complaining that the > remoteDeserializeTypedParameters() could allocate 'params', but > none of the callers could return the allocated memory back to their > caller since on input the param was passed by value. Additionally, > the flow of the code was that if params was NULL on entry, then each > function would return 'nparams' as the number of params entries the > caller would need to allocate in order to call the function again > with 'nparams' and 'params' being set. By the time the deserialize > routine was called params would have something. For other callers > where the 'params' was passed by reference as NULL since it's expected > that the deserialize allocates the memory and then have that passed > back to the original caller to dispose there was no Coverity issue. > > As it turns out Coverity didn't quite seem to understand the > relationship between 'nparams' and 'params'; however, if the > !userAllocated path of the deserialize code compared against > limit in any manner, then the Coverity error went away which > was quite strange, but useful. > > As it turns out one code path remoteDomainGetJobStats had a > comparison against 'limit' while another remoteConnectGetAllDomainStats > did not assuming that limit would be checked. So I refactored the > code a bit to cause the limit check to occur in deserialize for > both conditions and then only made the check of current returned > size against the incoming *nparams fail the non allocation case. > This means the job code doesn't need to check the limit any more, > while the stats code now does check the limit. > > Additionally, to help perhaps decipher which of the various > callers to the deserialize code caused the failure - I used > a #define to pass the __FUNCNAME__ of the caller along so that > error messages could have something like: > > error: remoteConnectGetAllDomainStats: too many parameters '2' for nparams '0' > error: Reconnected to the hypervisor > > (it's a contrived error just to show the funcname in the error) > > Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> > --- > src/remote/remote_driver.c | 49 ++++++++++++++++++++++++++-------------------- > 1 file changed, 28 insertions(+), 21 deletions(-) > ACK, Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list