[libvirt] [PATCH 0/9] More Coverity fixes

[John Ferlan]

On 09/11/2014 08:05 PM, John Ferlan wrote:
> There are two repeats from the last series (1 & 2).
> 
> For patch 1, I went with my suggestion - I'm open to others
> For patch 2, Coverity was complaining more about the way nparams
>     would be overwritten - fix that by adding a new variable
> 
> New patches
> 3 & 4 -> eblake helped out with these - especially the mgetgroups oddity
> 5 -> Fallout from fixing 4
> 6 -> virTimeFieldsThen() and the "offset = 0". I'd be OK with deleting the
>      code, but it just feels like someone had it on a todo list to come
>      back to some day
> 7 & 8 -> Fairly straightforward
> 9 -> This was an interesting case - it seems from what was being done
>      that I have the right "answer".  I did go all the way back to the
>      initial submission of the code and it did the same thing, except it
>      was using an unsigned long instead of int and well thus wouldn't
>      ever hit the condition since we're grabbing the big endian int value
> 
> This gets me down to 5 issues
> 
> John Ferlan (9):
>   remote_driver: Resolve Coverity RESOURCE_LEAK
>   virsh: Resolve Coverity NEGATIVE_RETURNS
>   daemon: Resolve Coverity RESOURCE_LEAK
>   virutil: Resolve Coverity RESOURCE_LEAK
>   virfile: Resolve Coverity RESOURCE_LEAK
>   virtime: Resolve Coverity DEADCODE
>   qemu: Resolve Coverity FORWARD_NULL
>   libxl: Resolve Coverity CHECKED_RETURN
>   virstoragefile: Resolve Coverity DEADCODE
> 
>  daemon/libvirtd.c            |  8 ++++----
>  src/libxl/libxl_driver.c     |  3 ++-
>  src/qemu/qemu_capabilities.c |  2 +-
>  src/remote/remote_driver.c   | 20 +++++++++++++++++++-
>  src/util/virfile.c           |  7 +++++--
>  src/util/virstoragefile.c    |  2 +-
>  src/util/virtime.c           |  2 ++
>  src/util/virutil.c           |  1 +
>  tools/virsh-domain.c         |  7 ++++---
>  9 files changed, 39 insertions(+), 13 deletions(-)
> 

I pushed 2-5,7,8 leaving 1, 6, & 9 for further examination/changes.

I went and looked deeper for 6 - the code was added by commit id
'3ec12898' when the logging API's needed to generate timestamps.  The
commit message has this:

 virTimeFieldsNowRaw  replacements for gmtime(), convert a timestamp
 virTimeFieldsThenRaw into a broken out set of fields. No localtime()
                      replacement is provided, because converting to
                      local time is not practical with only async signal
                      safe APIs.

So yeah - I think it's safe to remove the "deadcode", but will give it a
bit of (ahem) time for other viewpoints

Tks,

John