Libguestfs - Feb 2016 - Re: [PATCH 3/3] src: print contents of structs and struct lists on tracing

Extend the generator to generate a source (and the header for it) with
functions that print the content of a guestfs struct.  The code is
modelled after the code for it in fish.ml, although made a bit more
generic (destination FILE*, line separator) so it can be used also in
the library, when tracing.

This just introduces the new code and builds it as part of the helper
libutils.la.
---
 .gitignore        |   2 +
 generator/c.ml    | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 generator/c.mli   |   2 +
 generator/main.ml |   2 +
 po/POTFILES       |   1 +
 src/Makefile.am   |   6 ++-
 6 files changed, 136 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 40bebb3..1493011 100644
--- a/.gitignore
+++ b/.gitignore
@@ -468,6 +468,8 @@ Makefile.in
 /src/structs-compare.c
 /src/structs-copy.c
 /src/structs-free.c
+/src/structs-print.c
+/src/structs-print.h
 /src/test-utils
 /stamp-h1
 /sysprep/.depend
diff --git a/generator/c.ml b/generator/c.ml
index 417e2bc..9af4529 100644
--- a/generator/c.ml
+++ b/generator/c.ml
@@ -1204,6 +1204,130 @@ and generate_client_structs_cleanup ()  
   ) structs
 
+(* Generate structs-print.c file. *)
+and generate_client_structs_print_c () +  generate_header CStyle LGPLv2plus;
+
+  pr "\
+#include <config.h>
+
+#include <inttypes.h>
+
+#include \"c-ctype.h\"
+
+#include \"guestfs.h\"
+#include \"structs-print.h\"
+
+";
+
+  let write_structs +    List.iter (
+      fun { s_name = typ; s_cols = cols } ->
+        let needs_i +          List.exists (function (_, (FUUID|FBuffer)) ->
true | _ -> false) cols in
+
+        pr "void\n";
+        pr "guestfs_int_print_%s_indent (struct guestfs_%s *%s, FILE
*dest, const char *linesep, const char *indent)\n"
+          typ typ typ;
+        pr "{\n";
+        if needs_i then (
+          pr "  size_t i;\n";
+          pr "\n"
+        );
+        List.iter (
+          function
+          | name, FString ->
+              pr "  fprintf (dest, \"%%s%s: %%s%%s\", indent,
%s->%s, linesep);\n"
+                name typ name
+          | name, FUUID ->
+              pr "  fprintf (dest, \"%%s%s: \", indent);\n"
name;
+              pr "  for (i = 0; i < 32; ++i)\n";
+              pr "    fprintf (dest, \"%%c\",
%s->%s[i]);\n" typ name;
+              pr "  fprintf (dest, \"%%s\", linesep);\n"
+          | name, FBuffer ->
+              pr "  fprintf (dest, \"%%s%s: \", indent);\n"
name;
+              pr "  for (i = 0; i < %s->%s_len; ++i)\n" typ
name;
+              pr "    if (c_isprint (%s->%s[i]))\n" typ name;
+              pr "      fprintf (dest, \"%%c\",
%s->%s[i]);\n" typ name;
+              pr "    else\n";
+              pr "      fprintf (dest, \"\\\\x%%02x\",
(unsigned) %s->%s[i]);\n"
+                 typ name;
+              pr "  fprintf (dest, \"%%s\", linesep);\n"
+          | name, (FUInt64|FBytes) ->
+              pr "  fprintf (dest, \"%%s%s: %%\" PRIu64
\"%%s\", indent, %s->%s, linesep);\n"
+                name typ name
+          | name, FInt64 ->
+              pr "  fprintf (dest, \"%%s%s: %%\" PRIi64
\"%%s\", indent, %s->%s, linesep);\n"
+                name typ name
+          | name, FUInt32 ->
+              pr "  fprintf (dest, \"%%s%s: %%\" PRIu32
\"%%s\", indent, %s->%s, linesep);\n"
+                name typ name
+          | name, FInt32 ->
+              pr "  fprintf (dest, \"%%s%s: %%\" PRIi32
\"%%s\", indent, %s->%s, linesep);\n"
+                name typ name
+          | name, FChar ->
+              pr "  fprintf (dest, \"%%s%s: %%c%%s\", indent,
%s->%s, linesep);\n"
+                name typ name
+          | name, FOptPercent ->
+              pr "  if (%s->%s >= 0)\n" typ name;
+              pr "    fprintf (dest, \"%%s%s: %%g %%%%%%s\",
indent, (double) %s->%s, linesep);\n"
+                name typ name;
+              pr "  else\n";
+              pr "    fprintf (dest, \"%%s%s: %%s\", indent,
linesep);\n" name
+        ) cols;
+        pr "}\n";
+        pr "\n";
+    ) in
+
+  write_structs external_structs;
+
+  pr "\
+#if GUESTFS_PRIVATE
+
+";
+
+  write_structs internal_structs;
+
+  pr "\
+#endif /* End of GUESTFS_PRIVATE. */
+"
+
+(* Generate structs-print.h file. *)
+and generate_client_structs_print_h () +  generate_header CStyle LGPLv2plus;
+
+  pr "\
+#ifndef GUESTFS_INTERNAL_STRUCTS_PRINT_H_
+#define GUESTFS_INTERNAL_STRUCTS_PRINT_H_
+
+#include <stdio.h>
+
+";
+
+  let write_structs +    List.iter (
+      fun { s_name = name } ->
+        pr "extern void guestfs_int_print_%s_indent (struct guestfs_%s
*%s, FILE *dest, const char *linesep, const char *indent);\n"
+          name name name
+    ) in
+
+  write_structs external_structs;
+
+  pr "\
+
+#if GUESTFS_PRIVATE
+
+";
+
+  write_structs internal_structs;
+
+  pr "\
+
+#endif /* End of GUESTFS_PRIVATE. */
+
+#endif /* GUESTFS_INTERNAL_STRUCTS_PRINT_H_ */
+"
+
 (* Generate the client-side dispatch stubs. *)
 and generate_client_actions hash ()    generate_header CStyle LGPLv2plus;
diff --git a/generator/c.mli b/generator/c.mli
index 9a261a4..156b244 100644
--- a/generator/c.mli
+++ b/generator/c.mli
@@ -32,6 +32,8 @@ val generate_client_structs_cleanup : unit -> unit
 val generate_client_structs_compare : unit -> unit
 val generate_client_structs_copy : unit -> unit
 val generate_client_structs_free : unit -> unit
+val generate_client_structs_print_h : unit -> unit
+val generate_client_structs_print_c : unit -> unit
 val generate_event_string_c : unit -> unit
 val generate_guestfs_h : unit -> unit
 val generate_internal_actions_h : unit -> unit
diff --git a/generator/main.ml b/generator/main.ml
index 63a5d25..8d08d83 100644
--- a/generator/main.ml
+++ b/generator/main.ml
@@ -99,6 +99,8 @@ Run it from the top source directory using the command
   output_to "src/structs-copy.c" generate_client_structs_copy;
   output_to "src/structs-free.c" generate_client_structs_free;
   output_to "src/structs-cleanup.c" generate_client_structs_cleanup;
+  output_to "src/structs-print.c" generate_client_structs_print_c;
+  output_to "src/structs-print.h" generate_client_structs_print_h;
   output_to "src/actions-variants.c"
generate_client_actions_variants;
 
   for i = 0 to nr_actions_files-1 do
diff --git a/po/POTFILES b/po/POTFILES
index 2a1e313..0fb99b0 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -353,6 +353,7 @@ src/structs-cleanup.c
 src/structs-compare.c
 src/structs-copy.c
 src/structs-free.c
+src/structs-print.c
 src/test-utils.c
 src/tmpdirs.c
 src/utils.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 3ebb7f5..60641bf 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -46,7 +46,9 @@ generator_built = \
 	structs-cleanup.c \
 	structs-compare.c \
 	structs-copy.c \
-	structs-free.c
+	structs-free.c \
+	structs-print.c \
+	structs-print.h
 
 BUILT_SOURCES = \
 	$(generator_built) \
@@ -226,6 +228,8 @@ endif
 libutils_la_SOURCES = \
 	cleanup.c \
 	structs-cleanup.c \
+	structs-print.c \
+	structs-print.h \
 	utils.c
 libutils_la_CPPFLAGS = $(libguestfs_la_CPPFLAGS)
 libutils_la_CFLAGS = $(libguestfs_la_CFLAGS)
-- 
2.5.0

Use of the new code to print structs, making sure the layout is the same
as before.
---
 generator/fish.ml | 64 +++++--------------------------------------------------
 1 file changed, 5 insertions(+), 59 deletions(-)

diff --git a/generator/fish.ml b/generator/fish.ml
index c5bf7e0..0cbc781 100644
--- a/generator/fish.ml
+++ b/generator/fish.ml
@@ -94,6 +94,7 @@ let generate_fish_cmds ()    pr "\n";
   pr "#include \"guestfs.h\"\n";
   pr "#include \"guestfs-internal-frontend.h\"\n";
+  pr "#include \"structs-print.h\"\n";
   pr "\n";
   pr "#include \"fish.h\"\n";
   pr "#include \"fish-cmds.h\"\n";
@@ -255,70 +256,14 @@ Guestfish will prompt for these separately."
     pr "\n";
     pr "  for (i = 0; i < %ss->len; ++i) {\n" typ;
     pr "    printf (\"[%%zu] = {\\n\", i);\n";
-    pr "    print_%s_indent (&%ss->val[i], \" 
\");\n" typ typ;
+    pr "    guestfs_int_print_%s_indent (&%ss->val[i], stdout,
\"\\n\", \"  \");\n"
+      typ typ;
     pr "    printf (\"}\\n\");\n";
     pr "  }\n";
     pr "}\n";
     pr "\n";
   in
 
-  (* print_* functions *)
-  List.iter (
-    fun { s_name = typ; s_cols = cols } ->
-      let needs_i -        List.exists (function (_, (FUUID|FBuffer)) ->
true | _ -> false) cols in
-
-      pr "static void\n";
-      pr "print_%s_indent (struct guestfs_%s *%s, const char
*indent)\n" typ typ typ;
-      pr "{\n";
-      if needs_i then (
-        pr "  size_t i;\n";
-        pr "\n"
-      );
-      List.iter (
-        function
-        | name, FString ->
-            pr "  printf (\"%%s%s: %%s\\n\", indent,
%s->%s);\n" name typ name
-        | name, FUUID ->
-            pr "  printf (\"%%s%s: \", indent);\n" name;
-            pr "  for (i = 0; i < 32; ++i)\n";
-            pr "    printf (\"%%c\", %s->%s[i]);\n" typ
name;
-            pr "  printf (\"\\n\");\n"
-        | name, FBuffer ->
-            pr "  printf (\"%%s%s: \", indent);\n" name;
-            pr "  for (i = 0; i < %s->%s_len; ++i)\n" typ name;
-            pr "    if (c_isprint (%s->%s[i]))\n" typ name;
-            pr "      printf (\"%%c\", %s->%s[i]);\n"
typ name;
-            pr "    else\n";
-            pr "      printf (\"\\\\x%%02x\", (unsigned)
%s->%s[i]);\n"
-               typ name;
-            pr "  printf (\"\\n\");\n"
-        | name, (FUInt64|FBytes) ->
-            pr "  printf (\"%%s%s: %%\" PRIu64
\"\\n\", indent, %s->%s);\n"
-              name typ name
-        | name, FInt64 ->
-            pr "  printf (\"%%s%s: %%\" PRIi64
\"\\n\", indent, %s->%s);\n"
-              name typ name
-        | name, FUInt32 ->
-            pr "  printf (\"%%s%s: %%\" PRIu32
\"\\n\", indent, %s->%s);\n"
-              name typ name
-        | name, FInt32 ->
-            pr "  printf (\"%%s%s: %%\" PRIi32
\"\\n\", indent, %s->%s);\n"
-              name typ name
-        | name, FChar ->
-            pr "  printf (\"%%s%s: %%c\\n\", indent,
%s->%s);\n"
-              name typ name
-        | name, FOptPercent ->
-            pr "  if (%s->%s >= 0)\n" typ name;
-            pr "    printf (\"%%s%s: %%g %%%%\\n\", indent,
(double) %s->%s);\n"
-              name typ name;
-            pr "  else\n";
-            pr "    printf (\"%%s%s: \\n\", indent);\n"
name
-      ) cols;
-      pr "}\n";
-      pr "\n";
-  ) external_structs;
-
   (* Emit a print_TYPE_list function definition only if that function is used.
*)
   List.iter (
     function
@@ -335,7 +280,8 @@ Guestfish will prompt for these separately."
         pr "static void\n";
         pr "print_%s (struct guestfs_%s *%s)\n" typ typ typ;
         pr "{\n";
-        pr "  print_%s_indent (%s, \"\");\n" typ typ;
+        pr "  guestfs_int_print_%s_indent (%s, stdout, \"\\n\",
\"\");\n"
+          typ typ;
         pr "}\n";
         pr "\n";
     | typ, _ -> () (* empty *)
-- 
2.5.0

It eases the debugging, instead of getting just the name of the struct
returned.
---
 generator/c.ml | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/generator/c.ml b/generator/c.ml
index 9af4529..7446412 100644
--- a/generator/c.ml
+++ b/generator/c.ml
@@ -1348,6 +1348,7 @@ and generate_client_actions hash ()  #include
\"guestfs-internal-actions.h\"
 #include \"guestfs_protocol.h\"
 #include \"errnostring.h\"
+#include \"structs-print.h\"
 
 ";
 
@@ -1623,7 +1624,7 @@ and generate_client_actions hash ()  
     let needs_i        match ret with
-      | RStringList _ | RHashtable _ -> true
+      | RStringList _ | RHashtable _ | RStructList _ -> true
       | _ -> false in
     if needs_i then (
       pr "%s  size_t i;\n" indent;
@@ -1656,15 +1657,25 @@ and generate_client_actions hash ()           pr
"%s  }\n" indent;
          pr "%s  fputs (\"]\", trace_buffer.fp);\n" indent;
      | RStruct (_, typ) ->
-         (* XXX There is code generated for guestfish for printing
-          * these structures.  We need to make it generally available
-          * for all callers
-          *)
-         pr "%s  fprintf (trace_buffer.fp, \"<struct guestfs_%s
*>\");\n"
-           indent typ (* XXX *)
+         pr "%s  fprintf (trace_buffer.fp, \"<struct guestfs_%s =
\");\n"
+           indent typ;
+         pr "%s  guestfs_int_print_%s_indent (%s, trace_buffer.fp,
\", \", \"\");\n"
+           indent typ rv;
+         pr "%s  fprintf (trace_buffer.fp, \">\");\n"
indent
      | RStructList (_, typ) ->
-         pr "%s  fprintf (trace_buffer.fp, \"<struct
guestfs_%s_list *>\");\n"
-           indent typ (* XXX *)
+         pr "%s  fprintf (trace_buffer.fp, \"<struct
guestfs_%s_list(%%u)\", %s->len);\n"
+           indent typ rv;
+         pr "%s  if (%s->len > 0)\n" indent rv;
+         pr "%s    fprintf (trace_buffer.fp, \" = \");\n"
indent;
+         pr "%s  for (i = 0; i < %s->len; ++i) {\n" indent rv;
+         pr "%s    if (i != 0)\n" indent;
+         pr "%s      fprintf (trace_buffer.fp, \" \");\n"
indent;
+         pr "%s    fprintf (trace_buffer.fp, \"[%%zu]{\",
i);\n" indent;
+         pr "%s    guestfs_int_print_%s_indent (&%s->val[i],
trace_buffer.fp, \", \", \"\");\n"
+           indent typ rv;
+         pr "%s    fprintf (trace_buffer.fp, \"}\");\n"
indent;
+         pr "%s  }\n" indent;
+         pr "%s  fprintf (trace_buffer.fp, \">\");\n"
indent
     );
     pr "%s  guestfs_int_trace_send_line (g, &trace_buffer);\n"
indent;
     pr "%s}\n" indent;
-- 
2.5.0

On Wed, Feb 24, 2016 at 01:52:04PM +0100, Pino Toscano
wrote:
> It eases the debugging, instead of getting just the name of the struct
> returned.
The series is fine, so ACK.  Definitely improves the trace output :-)

A few things that would be good though:

 - Export the functions through the public API.  We already have
   functions to copy structs (eg. guestfs_copy_application,
   guestfs_copy_application_list).

 - Provide another function for printing lists of structs too, so you
   don't need the loop for RStructList.

 - With those two changes, you can get rid of the internal
   structs-print.h header.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top