Re: [PATCH 09/10] Ignore error returns for virBufferTrim().

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]