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:

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.