[libvirt] [PATCH v2 0/6] Fix some Coverity issues

[John Ferlan]

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