[libvirt] [PATCH 0/3] More Coverity changes

Michal Privoznik mprivozn at redhat.com
Thu Feb 25 15:34:37 UTC 2016


On 25.02.2016 15:21, John Ferlan wrote:
> All the issues here are found by a later version of Coverity than
> the one used by our internal testing. Two were false positives and
> one just an ordering issue with the virCheckFlags in the zfs backend.
> 
> The future coverity really has a problem with strtok_r usage after a
> VIR_STRDUP which only checks < 0 because it's perfectly fine for the
> "source" to be NULL and a < 0 check won't fail. The reason why this is
> a problem is the target is then NULL and used for the strtok_r call.
> Code inspection finds the cases we have are all false positives; however,
> rather than making the <= or sa_assert() type check to validate that -
> I chose to use the virStringSplitCount in order to split and inspect
> the input string. It was much cleaner at least on the inspection front.
> 
> The storage changes related to checking (ret == -1) resulted in a
> RESOURCE_LEAK false positive.  As it turns out, the only reason why
> -1 was being check was to determine whether or not the VIR_APPEND_ELEMENT
> insertion failed. If that succeeds, then 'vol' is cleared anyway, so
> truly we only need to attempt the free if we have a new vol. The
> virStorageVolDefFree will quietly return if vol == NULL.
> 
> John Ferlan (3):
>   openvz: Use virStringSplitCount instead of strtok_r
>   zfs: Resolve RESOURCE_LEAK
>   storage: No need to check ret after VIR_APPEND_ELEMENT
> 
>  src/openvz/openvz_conf.c              | 32 +++++++++++---------------------
>  src/storage/storage_backend_logical.c |  2 +-
>  src/storage/storage_backend_zfs.c     |  6 ++++--
>  3 files changed, 16 insertions(+), 24 deletions(-)
> 

ACK series and safe for the freeze. But see my comment for 1/3 before
pushing.

Michal




More information about the libvir-list mailing list