[Xen-devel] [PATCH] tools/libxl: Improvements to libxl-save-helper when using valgrind

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

Fix two unfree()'d allocations in libxl-save-helper, to get them out of the
way of other legitimate complains from valgrind.

The first is easy; close the interface to libxc when done with it.

The second requires quite a bit of code motion to fix sensibly.
 * The three logging functions are moved up.  The destroy() function has been
   modified to be less antisocial.
 * The global 'logger' is initialised in place. This requires changing the
   indirection of its use in 5 locations.
 * The struct xentoollog_logger_tellparent and function
   createlogger_tellparent() are unused and removed.

This completely removes any memory allocation associated with logging

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>

---

The macro XTL_NEW_LOGGER() is a special kind of crazy, requiring types and
functions with magic names to appear in scope.  It is also complete overkill
for libxl-save-helpers use.
---
 tools/libxl/libxl_save_helper.c |   87 +++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 50 deletions(-)

diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
index 880565e..7daff8e 100644
--- a/tools/libxl/libxl_save_helper.c
+++ b/tools/libxl/libxl_save_helper.c
@@ -47,10 +47,41 @@
 #include "xenguest.h"
 #include "_libxl_save_msgs_helper.h"
 
+/*----- logger -----*/
+static void tellparent_vmessage(xentoollog_logger *logger_in,
+                                xentoollog_level level,
+                                int errnoval,
+                                const char *context,
+                                const char *format,
+                                va_list al)
+{
+    char *formatted;
+    int r = vasprintf(&formatted, format, al);
+    if (r < 0) { perror("memory allocation failed during logging"); exit(-1); }
+    helper_stub_log(level, errnoval, context, formatted, 0);
+    free(formatted);
+}
+
+static void tellparent_progress(struct xentoollog_logger *logger_in,
+                                const char *context,
+                                const char *doing_what, int percent,
+                                unsigned long done, unsigned long total)
+{
+    helper_stub_progress(context, doing_what, done, total, 0);
+}
+
+static void tellparent_destroy(struct xentoollog_logger *logger_in)
+{
+}
+
 /*----- globals -----*/
 
 static const char *program = "libxl-save-helper";
-static xentoollog_logger *logger;
+static xentoollog_logger logger = {
+    tellparent_vmessage,
+    tellparent_progress,
+    tellparent_destroy
+ };
 static xc_interface *xch;
 
 /*----- error handling -----*/
@@ -61,7 +92,7 @@ static void fail(int errnoval, const char *fmt, ...)
 {
     va_list al;
     va_start(al,fmt);
-    xtl_logv(logger,XTL_ERROR,errnoval,program,fmt,al);
+    xtl_logv(&logger,XTL_ERROR,errnoval,program,fmt,al);
     exit(-1);
 }
 
@@ -86,45 +117,6 @@ static void *xmalloc(size_t sz)
     return r;
 }
 
-/*----- logger -----*/
-
-typedef struct {
-    xentoollog_logger vtable;
-} xentoollog_logger_tellparent;
-
-static void tellparent_vmessage(xentoollog_logger *logger_in,
-                                xentoollog_level level,
-                                int errnoval,
-                                const char *context,
-                                const char *format,
-                                va_list al)
-{
-    char *formatted;
-    int r = vasprintf(&formatted, format, al);
-    if (r < 0) { perror("memory allocation failed during logging"); exit(-1); }
-    helper_stub_log(level, errnoval, context, formatted, 0);
-    free(formatted);
-}
-
-static void tellparent_progress(struct xentoollog_logger *logger_in,
-                                const char *context,
-                                const char *doing_what, int percent,
-                                unsigned long done, unsigned long total)
-{
-    helper_stub_progress(context, doing_what, done, total, 0);
-}
-
-static void tellparent_destroy(struct xentoollog_logger *logger_in)
-{
-    abort();
-}
-
-static xentoollog_logger_tellparent *createlogger_tellparent(void)
-{
-    xentoollog_logger_tellparent newlogger;
-    return XTL_NEW_LOGGER(tellparent, newlogger);
-}
-
 /*----- helper functions called by autogenerated stubs -----*/
 
 unsigned char * helper_allocbuf(int len, void *user)
@@ -184,22 +176,17 @@ static int toolstack_save_cb(uint32_t domid, uint8_t 
**buf,
 }
 
 static void startup(const char *op) {
-    logger = (xentoollog_logger*)createlogger_tellparent();
-    if (!logger) {
-        fprintf(stderr, "%s: cannot initialise logger\n", program);
-        exit(-1);
-    }
-
-    xtl_log(logger,XTL_DEBUG,0,program,"starting %s",op);
+    xtl_log(&logger,XTL_DEBUG,0,program,"starting %s",op);
 
-    xch = xc_interface_open(logger,logger,0);
+    xch = xc_interface_open(&logger,&logger,0);
     if (!xch) fail(errno,"xc_interface_open failed");
 }
 
 static void complete(int retval) {
     int errnoval = retval ? errno : 0; /* suppress irrelevant errnos */
-    xtl_log(logger,XTL_DEBUG,errnoval,program,"complete r=%d",retval);
+    xtl_log(&logger,XTL_DEBUG,errnoval,program,"complete r=%d",retval);
     helper_stub_complete(retval,errnoval,0);
+    xc_interface_close(xch);
     exit(0);
 }
 
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel