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