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