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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list