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