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

Re: [libvirt] [PATCH] check return values of virBufferTrim



On 06/17/2013 02:15 PM, Michal Privoznik wrote:
> On 17.06.2013 10:34, Ján Tomko wrote:
>> Just to silence Coverity:
>>
>> Event check_return:
>> Calling function "virBufferTrim(virBufferPtr, char const *, int)"
>> without checking return value (as is done elsewhere 5 out of 6 times).
>> ---
>>  src/node_device/node_device_udev.c | 5 ++---
>>  src/rpc/virnetsshsession.c         | 3 +--
>>  2 files changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
>> index bb58415..a37989a 100644
>> --- a/src/node_device/node_device_udev.c
>> +++ b/src/node_device/node_device_udev.c
>> @@ -370,9 +370,8 @@ udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED,
>>      const char *format = NULL;
>>  
>>      virBufferAdd(&buf, fmt, -1);
>> -    virBufferTrim(&buf, "\n", -1);
>> -
>> -    format = virBufferContentAndReset(&buf);
>> +    if (virBufferTrim(&buf, "\n", -1) >= 0)
>> +        format = virBufferContentAndReset(&buf);
>>  
>>      virLogVMessage(VIR_LOG_FROM_LIBRARY,
>>                     virLogPriorityFromSyslog(priority),
>> diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c
>> index b6aedc8..2299871 100644
>> --- a/src/rpc/virnetsshsession.c
>> +++ b/src/rpc/virnetsshsession.c
>> @@ -362,9 +362,8 @@ virNetSSHCheckHostKey(virNetSSHSessionPtr sess)
>>               * we have to use a *MAGIC* constant. */
>>              for (i = 0; i < 16; i++)
>>                      virBufferAsprintf(&buff, "%02hhX:", keyhash[i]);
>> -            virBufferTrim(&buff, ":", 1);
>>  
>> -            if (virBufferError(&buff) != 0) {
>> +            if (virBufferTrim(&buff, ":", 1) < 0) {
>>                  virReportOOMError();
>>                  return -1;
>>              }
>>
> 
> Shouldn't we make virBufferTrim call virBufferSetError instead? I think
> it's a better approach, as it fits to calling scheme of other virBuffer*
> functions better:

Is it really an error if you can't find a string to trim?
Or we could introduce another version of virBufferTrim, without a return
value, to use anywhere we don't care about the return value (i.e. outside the
tests :))

Jan


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