On 01/12/2015 02:36 AM, Pavel Hrdina wrote:
On 01/09/2015 05:02 PM, John Ferlan wrote:
With the build fix, my coverity build worked again (good), but found more issues (boo).
Consider these changes partially as a replacement for Pavel's patch:
http://www.redhat.com/archives/libvir-list/2015-January/msg00240.html
with some additional changes for checks that need to be made in failure path scenario. I didn't make any of the code formatting changes Pavel had made for virDomainNetIpsFormat, but I can do that. I just wanted to get these on the list for a comparison. Additionally, Pavel's change for virDomainNetRoutesFormat didn't take into account that the alloc's are done in a for loop, so both the VIR_FREE's needed to be done in the loop.
There are still some Coverity issues left over after this, but I'm not quite sure how to fix them, hence the reply on the Xen-xl parser:
http://www.redhat.com/archives/libvir-list/2015-January/msg00243.html
John Ferlan (6): openvz: Resolve Coverity RESOURCE_LEAK openvz: Check errors from virSocketAddrFormat domain_conf: Resolve Coverity RESOURCE_LEAK domain_conf: Check errors from virSocketAddrFormat xenconfig: Resolve Coverity RESOURCE_LEAK virconf: Resolve a possible memory leak in virConfSetValue
src/conf/domain_conf.c | 32 ++++++++++++++++++++++++-------- src/openvz/openvz_driver.c | 3 +++ src/util/virconf.c | 4 +++- src/xenconfig/xen_xl.c | 6 +++--- 4 files changed, 33 insertions(+), 12 deletions(-)
Oh, thanks for noticing that the code was ignoring return values. I've just blindly looked at the memory leaks and fixed them without wondering if there is anything else wrong.
ACK to the series except the 5/6 patch for xen_xl parser as there is a new series waiting for review which will revert the code and introduce a new approach to parse the xen xl configuration.
OK - thanks for the review. However, I think I'm going to wait a few cycles before addressing 3/6 & 4/6 since it seems Cedric Bosdonnat is already in the process of adjusting virDomainNetRoutesFormat() and these changes may partially be duplicitous. Cedric - perhaps as part of your changes, you could also make adjustments to cover the cases found through Coverity using the two domain_conf.c patches I've posted to resolve Coverity issues? I believe they were issues as a result of the 15 patch series pushed recently. I will push 1, 2, and 6 shortly. John