[libvirt] [PATCH] check return values of virBufferTrim

Michal Privoznik mprivozn at redhat.com
Mon Jun 17 12:15:17 UTC 2013


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:

virBuffer buf = VIR_BUFFER_INITIALIZER;

virBufferSomething(&buf, ...);
virBufferSomethingElse(&buf, ...);

if (virBufferError(&buf) != 0) {
   virReportError(...);
} else {
   char *tmp = virBufferContentAndReset(&buf);
   ...
}

I guess it will shut the coverity up as well.

Michal




More information about the libvir-list mailing list