Re: [libvirt] [PATCH 0/6] Resolve some Coverity errors and other issues found

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.