4:08 a.m.
On Sun, Jun 03, 2018 at 01:42:05PM +0530, Sukrit Bhatnagar wrote:
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virbuffer.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index 3d6defb..5152f73 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -456,7 +456,8 @@ void
virBufferEscapeString(virBufferPtr buf, const char *format, const char *str)
{
int len;
- char *escaped, *out;
+ VIR_AUTOFREE(char *) escaped = NULL;
+ char *out;
const char *cur;
const char forbidden_characters[] = {
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
@@ -533,7 +534,6 @@ virBufferEscapeString(virBufferPtr buf, const char *format, const
char *str)
*out = 0;
virBufferAsprintf(buf, format, escaped);
- VIR_FREE(escaped);
}
/**
@@ -612,7 +612,8 @@ virBufferEscape(virBufferPtr buf, char escape, const char *toescape,
const char *format, const char *str)
{
int len;
- char *escaped, *out;
+ VIR_AUTOFREE(char *) escaped = NULL;
+ char *out;
const char *cur;
if ((format == NULL) || (buf == NULL) || (str == NULL))
@@ -644,7 +645,6 @@ virBufferEscape(virBufferPtr buf, char escape, const char *toescape,
*out = 0;
virBufferAsprintf(buf, format, escaped);
- VIR_FREE(escaped);
}
@@ -675,11 +675,11 @@ virBufferEscapeN(virBufferPtr buf,
{
int len;
size_t i;
- char *escaped = NULL;
+ VIR_AUTOFREE(char *) escaped = NULL;
char *out;
const char *cur;
struct _virBufferEscapePair escapeItem;
- struct _virBufferEscapePair *escapeList = NULL;
+ VIR_AUTOFREE(struct _virBufferEscapePair *) escapeList = NULL;
During the design discussion, I said I was fine with ^this usage and I still
kinda am, but at the same time Andrea argued in the same thread [1] that we
should stay consistent here in terms that every virSomething should have a
corresponding virSomethingFree method even if it ends up just calling VIR_FREE.
And he's right in that it could easily turn against us, simply because it can
look a bit fishy that we're calling plain VIR_FREE on the structure pointer
rather than having a dedicated cleanup for the compound type at first glance, so
you have to double check whether it's really okay to use it this way.
So unless someone else comes and says that they're absolutely fine with making
exceptions like this (although consistency is key to a readable and
maintainable code), I'd say let's shoot for consistency, create a static
virBufferEscapePairFree helper and declare VIR_AUTOFREE as a macro taking care
of automatic freeing of *scalar* resources.
[1] https://www.redhat.com/archives/libvir-list/2018-May/msg01910.html
size_t nescapeList = 0;
va_list ap;
@@ -696,7 +696,8 @@ virBufferEscapeN(virBufferPtr buf,
while ((escapeItem.escape = va_arg(ap, int))) {
if (!(escapeItem.toescape = va_arg(ap, char *))) {
virBufferSetError(buf, errno);
- goto cleanup;
+ va_end(ap);
+ return;
}
if (strcspn(str, escapeItem.toescape) == len)
@@ -704,19 +705,22 @@ virBufferEscapeN(virBufferPtr buf,
if (VIR_APPEND_ELEMENT_QUIET(escapeList, nescapeList, escapeItem) < 0) {
virBufferSetError(buf, errno);
- goto cleanup;
+ va_end(ap);
+ return;
so we basically replaced one goto statement for 2 statements where the va_end
statement is very easily to be forgotten... the cleanup label should stay in
place here, calling va_end and return since the frees will be taken care of.
Erik
}
}
if (nescapeList == 0) {
virBufferAsprintf(buf, format, str);
- goto cleanup;
+ va_end(ap);
+ return;
}
if (xalloc_oversized(2, len) ||
VIR_ALLOC_N_QUIET(escaped, 2 * len + 1) < 0) {
virBufferSetError(buf, errno);
- goto cleanup;
+ va_end(ap);
+ return;
}
cur = str;
@@ -734,11 +738,6 @@ virBufferEscapeN(virBufferPtr buf,
*out = 0;
virBufferAsprintf(buf, format, escaped);
-
- cleanup:
- va_end(ap);
- VIR_FREE(escapeList);
- VIR_FREE(escaped);
}
@@ -803,7 +802,8 @@ void
virBufferEscapeShell(virBufferPtr buf, const char *str)
{
int len;
- char *escaped, *out;
+ VIR_AUTOFREE(char *) escaped = NULL;
+ char *out;
const char *cur;
if ((buf == NULL) || (str == NULL))
@@ -847,7 +847,6 @@ virBufferEscapeShell(virBufferPtr buf, const char *str)
*out = 0;
virBufferAdd(buf, escaped, -1);
- VIR_FREE(escaped);
}
/**
--
1.8.3.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list