On 09/25/2015 12:31 PM, John Ferlan wrote:
This series is based off the review of patch 1 from the series:
http://www.redhat.com/archives/libvir-list/2015-September/msg00841.html
In review of patch 1:
http://www.redhat.com/archives/libvir-list/2015-September/msg00859.html
it was noted that instead of using sa_assert, the proper checks should be made. During investigation, I found that while the caller could check for a non-NULL "first" parameter that ends up being used for strtok_r, that was not "good enough" for Coverity which still needed to consider the function where the to be first param cannot be NULL.
In any case, I separated out each into their own patch rather than lumping them together.
Patches 1-4 should be relatively straightforward.
Patch 5 is new - it's one that I had been working on and finally figured out what the issue is/was. It was a bit more complex and hidden.
Patch 6 was from the original patch 1, but it's review had a comment regarding using virBitmap* instead of the open coding. This one I believe I have intoned the magic words to make it better, but since I don't use xenapi, perhaps extra care would be necessary to make sure I got it right.
John Ferlan (6): openvz: Resolve Coverity FORWARD_NULL openvz: Resolve Coverity FORWARD_NULL libxl: Resolve Coverity FORWARD_NULL esx: Resolve Coverity FORWARD_NULL qemu: Resolve Coverity FORWARD_NULL xenapi: Resolve Coverity FORWARD_NULL
src/esx/esx_vi.c | 5 +++++ src/libxl/libxl_conf.c | 6 ++++++ src/openvz/openvz_conf.c | 8 ++++---- src/qemu/qemu_process.c | 14 ++++++++++++-- src/xenapi/xenapi_driver.c | 12 +++++++----- src/xenapi/xenapi_utils.c | 21 --------------------- src/xenapi/xenapi_utils.h | 2 -- 7 files changed, 34 insertions(+), 34 deletions(-)
I pushed patches 3 & 5 (with adjustment noted in review to code and commit message). Patch 1, 2, and 4 I understand are NACK'd - that's fine - I get the reasoning. Not sure I 100% agree with the statement that we wouldn't accept a patch that wasn't absolutely necessary or didn't help. I find some of the refactor patches unnecessary especially when they mess up backports, but they get accepted for the 'greater good'. Patch 6 while ACK'd I'm less confident about the results. It would be nice to have someone with the xenapi environment "test" that it works. Since it too is a "workaround" of sorts similar to patches 1, 2, & 4, I'll let it sit for now. I finally had some luck generating a small program to exhibit the error - I'll be able to submit a coverity bug and see what happens. John