We don't care whether the trim was succesful or not anywhere except the tests. Switch it to void and set the buffer error on wrong usage. --- Patch that tried to shut Coverity up by checking the return values anyway: https://www.redhat.com/archives/libvir-list/2013-June/msg00650.html src/util/virbuffer.c | 19 ++++++++++--------- src/util/virbuffer.h | 2 +- tests/virbuftest.c | 43 ++++++++++++++++--------------------------- tools/virsh.c | 9 +++------ 4 files changed, 30 insertions(+), 43 deletions(-) diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index 693e4b2..4969497 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -660,27 +660,28 @@ virBufferStrcat(virBufferPtr buf, ...) * further limits how much of the tail is trimmed. If @str is NULL, then * @len must be non-negative. * - * Returns -1 if @buf has previously encountered an error or if @len is - * invalid, 0 if there was nothing to trim (@buf was too short or @str - * didn't match), and 1 if the trim was successful. + * Sets error to -1 (usage) if str is NULL and len is less than zero. */ -int +void virBufferTrim(virBufferPtr buf, const char *str, int len) { size_t len2 = 0; - if (!buf || buf->error || (!str && len < 0)) - return -1; + if (!buf || buf->error) + return; + if (!str && len < 0) { + virBufferSetError(buf, -1); + return; + } if (len > 0 && len > buf->use) - return 0; + return; if (str) { len2 = strlen(str); if (len2 > buf->use || memcmp(&buf->content[buf->use - len2], str, len2) != 0) - return 0; + return; } buf->use -= len < 0 ? len2 : len; buf->content[buf->use] = '\0'; - return 1; } diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h index 7b69c0e..e0b77ab 100644 --- a/src/util/virbuffer.h +++ b/src/util/virbuffer.h @@ -77,6 +77,6 @@ void virBufferURIEncodeString(virBufferPtr buf, const char *str); void virBufferAdjustIndent(virBufferPtr buf, int indent); int virBufferGetIndent(const virBufferPtr buf, bool dynamic); -int virBufferTrim(virBufferPtr buf, const char *trim, int len); +void virBufferTrim(virBufferPtr buf, const char *trim, int len); #endif /* __VIR_BUFFER_H__ */ diff --git a/tests/virbuftest.c b/tests/virbuftest.c index c7a504b..3938f0d 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -148,38 +148,21 @@ static int testBufTrim(const void *data ATTRIBUTE_UNUSED) char *result = NULL; const char *expected = "a,b"; int ret = -1; - int i = 1; - -#define ACT(str, len, result) \ - do { \ - if (virBufferTrim(buf, str, len) != result) { \ - TEST_ERROR("trim %d failed", i); \ - goto cleanup; \ - } \ - i++; \ - } while (0); - - if (virBufferTrim(buf, "", 0) != -1) { - TEST_ERROR("Wrong failure detection 1"); - goto cleanup; - } + + virBufferTrim(buf, "", 0); buf = &bufinit; - if (virBufferTrim(buf, NULL, -1) != -1) { - TEST_ERROR("Wrong failure detection 2"); - goto cleanup; - } virBufferAddLit(buf, "a;"); - ACT("", 0, 1); - ACT("", -1, 1); - ACT(NULL, 1, 1); - ACT(NULL, 5, 0); - ACT("a", 2, 0); + virBufferTrim(buf, "", 0); + virBufferTrim(buf, "", -1); + virBufferTrim(buf, NULL, 1); + virBufferTrim(buf, NULL, 5); + virBufferTrim(buf, "a", 2); virBufferAddLit(buf, ",b,,"); - ACT("b", -1, 0); - ACT("b,,", 1, 1); - ACT(",", -1, 1); + virBufferTrim(buf, "b", -1); + virBufferTrim(buf, "b,,", 1); + virBufferTrim(buf, ",", -1); result = virBufferContentAndReset(buf); if (!result || STRNEQ(result, expected)) { @@ -187,6 +170,12 @@ static int testBufTrim(const void *data ATTRIBUTE_UNUSED) goto cleanup; } + virBufferTrim(buf, NULL, -1); + if (virBufferError(buf) != -1) { + TEST_ERROR("Usage error not flagged"); + goto cleanup; + } + ret = 0; cleanup: diff --git a/tools/virsh.c b/tools/virsh.c index 26d37c6..af2bb76 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -632,18 +632,15 @@ vshTreePrintInternal(vshControl *ctl, false, indent) < 0) goto cleanup; } - if (virBufferTrim(indent, " ", -1) < 0) - goto cleanup; + virBufferTrim(indent, " ", -1); /* If there was no child device, and we're the last in * a list of devices, then print another blank line */ if (nextlastdev == -1 && devid == lastdev) vshPrint(ctl, "%s\n", virBufferCurrentContent(indent)); - if (!root) { - if (virBufferTrim(indent, NULL, 2) < 0) - goto cleanup; - } + if (!root) + virBufferTrim(indent, NULL, 2); ret = 0; cleanup: return ret; -- 1.8.1.5