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