On 01/04/2013 12:50 PM, Eric Blake wrote:
On 01/03/2013 12:39 PM, Daniel P. Berrange wrote:
On Thu, Jan 03, 2013 at 02:16:21PM -0500, John Ferlan wrote:
--- tools/virsh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
I must say I don't see the point in the return value for virBufferTrim. I think I'd recommend turning it into a 'void' function
Ah, but virbuftest.c exposes why a return value can be useful - it lets you know how much was trimmed, or if trimming was prevented because the string didn't end with the suffix you expect. Making the function 'void' would require rewriting the test, and losing out on that functionality. Just because all current callers outside of the testsuite don't use that functionality (virnetsshsession.c, virsh.c) doesn't mean we should necessarily get rid of it - is there any alternative way to shut up Coverity to say that we can safely ignore this return value without having to mark up all the callers?
Coverity allows one to place a comment in the code prior to the call to signify to the analysis engine to ignore a particular issue or a set of particular issues, such as in these cases: /* coverity[check_return] */ I already have a list of those starting in a local branch. In this particular case I chose ignore_value because the code is merely removing what was added to indent in prior lines. For the "(!root)" case if we failed to add, then the code jumps to cleanup. I suppose an argument could be made that if either fails we could print some sort of error, but it just seemed unnecessary since I believe whatever was going to be printed was already printed. The indent is only used to create a bit more beatification it seems. John