[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