Libguestfs - Oct 2017 - [PATCH miniexpect 0/2] Add debugging capability at runtime.

Currently you can only turn on miniexpect debugging by recompiling.
These two patches make it configurable at runtime, and also improve
the usefulness of the output.

Rich.

---
 miniexpect.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/miniexpect.c b/miniexpect.c
index d5a7c6e..d2ceb69 100644
--- a/miniexpect.c
+++ b/miniexpect.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include <stdarg.h>
 #include <string.h>
+#include <ctype.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <signal.h>
@@ -47,6 +48,10 @@
 
 #define DEBUG 0
 
+#if DEBUG
+static void debug_buffer (FILE *, char *);
+#endif
+
 static mexp_h *
 create_handle (void)
 {
@@ -333,7 +338,9 @@ mexp_expect (mexp_h *h, const mexp_regexp *regexps, int
*ovector, int ovecsize)
     h->buffer[h->len] = '\0';
 #if DEBUG
     fprintf (stderr, "DEBUG: read %zd bytes from pty\n", rs);
-    fprintf (stderr, "DEBUG: buffer content: %s\n", h->buffer);
+    fprintf (stderr, "DEBUG: buffer content: ");
+    debug_buffer (stderr, h->buffer);
+    fprintf (stderr, "\n");
 #endif
 
   try_match:
@@ -406,7 +413,9 @@ mexp_printf (mexp_h *h, const char *fs, ...)
     return -1;
 
 #if DEBUG
-  fprintf (stderr, "DEBUG: writing: %s\n", msg);
+  fprintf (stderr, "DEBUG: writing: ");
+  debug_buffer (stderr, msg);
+  fprintf (stderr, "\n");
 #endif
 
   n = len;
@@ -430,3 +439,31 @@ mexp_send_interrupt (mexp_h *h)
 {
   return write (h->fd, "\003", 1);
 }
+
+#if DEBUG
+/* Print escaped buffer to fp. */
+static void
+debug_buffer (FILE *fp, char *buf)
+{
+  while (*buf) {
+    if (isprint (*buf))
+      fputc (*buf, fp);
+    else {
+      switch (*buf) {
+      case '\0': fputs ("\\0", fp); break;
+      case '\a': fputs ("\\a", fp); break;
+      case '\b': fputs ("\\b", fp); break;
+      case '\f': fputs ("\\f", fp); break;
+      case '\n': fputs ("\\n", fp); break;
+      case '\r': fputs ("\\r", fp); break;
+      case '\t': fputs ("\\t", fp); break;
+      case '\v': fputs ("\\v", fp); break;
+      default:
+        fprintf (fp, "\\x%x", (unsigned char) *buf);
+      }
+    }
+    buf++;
+  }
+}
+#endif
+
-- 
2.13.2

Debugging can now be enabled at runtime.
---
 example-sshpass.c | 46 +++++++++++++++++++++++++-------
 miniexpect.c      | 78 ++++++++++++++++++++++++++++++++++---------------------
 miniexpect.h      |  6 +++++
 miniexpect.pod    | 21 +++++++++++++++
 4 files changed, 113 insertions(+), 38 deletions(-)

diff --git a/example-sshpass.c b/example-sshpass.c
index 3449a20..11df02e 100644
--- a/example-sshpass.c
+++ b/example-sshpass.c
@@ -24,8 +24,10 @@
  * The remaining arguments are passed to the ssh subprocess.
  *
  * For example:
- *   sshpass 123456 ssh remote.example.com
- *   sshpass 123456 ssh -l root remote.example.com
+ *   sshpass [-d] 123456 ssh remote.example.com
+ *   sshpass [-d] 123456 ssh -l root remote.example.com
+ *
+ * Use the -d flag to enable debugging to stderr.
  */
 
 #include <config.h>
@@ -42,9 +44,18 @@
 
 static pcre *compile_re (const char *rex);
 
+static void
+usage (void)
+{
+  fprintf (stderr, "usage: sshpass PASSWORD ssh [SSH-ARGS...]
HOST\n");
+  exit (EXIT_FAILURE);
+}
+
 int
 main (int argc, char *argv[])
 {
+  int opt;
+  int debug = 0;
   mexp_h *h;
   const char *password;
   int status;
@@ -52,20 +63,31 @@ main (int argc, char *argv[])
   const int ovecsize = 12;
   int ovector[ovecsize];
 
-  if (argc <= 3) {
-    fprintf (stderr, "usage: sshpass PASSWORD ssh [SSH-ARGS...]
HOST\n");
-    exit (EXIT_FAILURE);
+  while ((opt = getopt (argc, argv, "d")) != -1) {
+    switch (opt) {
+    case 'd':
+      debug = 1;
+      break;
+    default:
+      usage ();
+    }
   }
 
-  password = argv[1];
+  if (argc-optind <= 2)
+    usage ();
+
+  password = argv[optind];
+  optind++;
 
   printf ("starting ssh command ...\n");
 
-  h = mexp_spawnv (argv[2], &argv[2]);
+  h = mexp_spawnv (argv[optind], &argv[optind]);
   if (h == NULL) {
     perror ("mexp_spawnv: ssh");
     exit (EXIT_FAILURE);
   }
+  if (debug)
+    mexp_set_debug_file (h, stderr);
 
   /* Wait for the password prompt. */
   password_re = compile_re ("assword");
@@ -91,10 +113,16 @@ main (int argc, char *argv[])
     goto error;
   }
 
-  /* Got the password prompt, so send a password. */
+  /* Got the password prompt, so send a password.
+   *
+   * Note use of mexp_printf_password here which is identical to
+   * mexp_printf except that it hides the password in debugging
+   * output.
+   */
   printf ("sending the password ...\n");
 
-  if (mexp_printf (h, "%s\n", password) == -1) {
+  if (mexp_printf_password (h, "%s", password) == -1 ||
+      mexp_printf (h, "\n") == -1) {
     perror ("mexp_printf");
     goto error;
   }
diff --git a/miniexpect.c b/miniexpect.c
index d2ceb69..1ac373b 100644
--- a/miniexpect.c
+++ b/miniexpect.c
@@ -46,11 +46,7 @@
 
 #include "miniexpect.h"
 
-#define DEBUG 0
-
-#if DEBUG
 static void debug_buffer (FILE *, char *);
-#endif
 
 static mexp_h *
 create_handle (void)
@@ -68,6 +64,7 @@ create_handle (void)
   h->buffer = NULL;
   h->len = h->alloc = 0;
   h->next_match = -1;
+  h->debug_fp = NULL;
   h->user1 = h->user2 = h->user3 = NULL;
 
   return h;
@@ -296,9 +293,8 @@ mexp_expect (mexp_h *h, const mexp_regexp *regexps, int
*ovector, int ovecsize)
     pfds[0].events = POLLIN;
     pfds[0].revents = 0;
     r = poll (pfds, 1, timeout);
-#if DEBUG
-    fprintf (stderr, "DEBUG: poll returned %d\n", r);
-#endif
+    if (h->debug_fp)
+      fprintf (h->debug_fp, "DEBUG: poll returned %d\n", r);
     if (r == -1)
       return MEXP_ERROR;
 
@@ -318,9 +314,8 @@ mexp_expect (mexp_h *h, const mexp_regexp *regexps, int
*ovector, int ovecsize)
       h->alloc += h->read_size;
     }
     rs = read (h->fd, h->buffer + h->len, h->read_size);
-#if DEBUG
-    fprintf (stderr, "DEBUG: read returned %zd\n", rs);
-#endif
+    if (h->debug_fp)
+      fprintf (h->debug_fp, "DEBUG: read returned %zd\n", rs);
     if (rs == -1) {
       /* Annoyingly on Linux (I'm fairly sure this is a bug) if the
        * writer closes the connection, the entire pty is destroyed,
@@ -336,12 +331,12 @@ mexp_expect (mexp_h *h, const mexp_regexp *regexps, int
*ovector, int ovecsize)
     /* We read something. */
     h->len += rs;
     h->buffer[h->len] = '\0';
-#if DEBUG
-    fprintf (stderr, "DEBUG: read %zd bytes from pty\n", rs);
-    fprintf (stderr, "DEBUG: buffer content: ");
-    debug_buffer (stderr, h->buffer);
-    fprintf (stderr, "\n");
-#endif
+    if (h->debug_fp) {
+      fprintf (h->debug_fp, "DEBUG: read %zd bytes from pty\n",
rs);
+      fprintf (h->debug_fp, "DEBUG: buffer content: ");
+      debug_buffer (h->debug_fp, h->buffer);
+      fprintf (h->debug_fp, "\n");
+    }
 
   try_match:
     /* See if there is a full or partial match against any regexp. */
@@ -395,28 +390,32 @@ mexp_expect (mexp_h *h, const mexp_regexp *regexps, int
*ovector, int ovecsize)
   }
 }
 
-int
-mexp_printf (mexp_h *h, const char *fs, ...)
+static int mexp_vprintf (mexp_h *h, int password, const char *fs, va_list args)
+  __attribute__((format(printf,3,0)));
+
+static int
+mexp_vprintf (mexp_h *h, int password, const char *fs, va_list args)
 {
-  va_list args;
   char *msg;
   int len;
   size_t n;
   ssize_t r;
   char *p;
 
-  va_start (args, fs);
   len = vasprintf (&msg, fs, args);
-  va_end (args);
 
   if (len < 0)
     return -1;
 
-#if DEBUG
-  fprintf (stderr, "DEBUG: writing: ");
-  debug_buffer (stderr, msg);
-  fprintf (stderr, "\n");
-#endif
+  if (h->debug_fp) {
+    if (!password) {
+      fprintf (h->debug_fp, "DEBUG: writing: ");
+      debug_buffer (h->debug_fp, msg);
+      fprintf (h->debug_fp, "\n");
+    }
+    else
+      fprintf (h->debug_fp, "DEBUG: writing the password\n");
+  }
 
   n = len;
   p = msg;
@@ -435,12 +434,35 @@ mexp_printf (mexp_h *h, const char *fs, ...)
 }
 
 int
+mexp_printf (mexp_h *h, const char *fs, ...)
+{
+  int r;
+  va_list args;
+
+  va_start (args, fs);
+  r = mexp_vprintf (h, 0, fs, args);
+  va_end (args);
+  return r;
+}
+
+int
+mexp_printf_password (mexp_h *h, const char *fs, ...)
+{
+  int r;
+  va_list args;
+
+  va_start (args, fs);
+  r = mexp_vprintf (h, 1, fs, args);
+  va_end (args);
+  return r;
+}
+
+int
 mexp_send_interrupt (mexp_h *h)
 {
   return write (h->fd, "\003", 1);
 }
 
-#if DEBUG
 /* Print escaped buffer to fp. */
 static void
 debug_buffer (FILE *fp, char *buf)
@@ -465,5 +487,3 @@ debug_buffer (FILE *fp, char *buf)
     buf++;
   }
 }
-#endif
-
diff --git a/miniexpect.h b/miniexpect.h
index e4d6010..14d8236 100644
--- a/miniexpect.h
+++ b/miniexpect.h
@@ -29,6 +29,7 @@
 #ifndef MINIEXPECT_H_
 #define MINIEXPECT_H_
 
+#include <stdio.h>
 #include <unistd.h>
 
 #include <pcre.h>
@@ -44,6 +45,7 @@ struct mexp_h {
   ssize_t next_match;
   size_t read_size;
   int pcre_error;
+  FILE *debug_fp;
   void *user1;
   void *user2;
   void *user3;
@@ -62,6 +64,8 @@ typedef struct mexp_h mexp_h;
 #define mexp_get_read_size(h) ((h)->read_size)
 #define mexp_set_read_size(h, size) ((h)->read_size = (size))
 #define mexp_get_pcre_error(h) ((h)->pcre_error)
+#define mexp_set_debug_file(h, fp) ((h)->debug_fp = (fp))
+#define mexp_get_debug_file(h) ((h)->debug_fp)
 
 /* Spawn a subprocess. */
 extern mexp_h *mexp_spawnvf (unsigned flags, const char *file, char **argv);
@@ -99,6 +103,8 @@ extern int mexp_expect (mexp_h *h, const mexp_regexp
*regexps,
 /* Sending commands, keypresses. */
 extern int mexp_printf (mexp_h *h, const char *fs, ...)
   __attribute__((format(printf,2,3)));
+extern int mexp_printf_password (mexp_h *h, const char *fs, ...)
+  __attribute__((format(printf,2,3)));
 extern int mexp_send_interrupt (mexp_h *h);
 
 #endif /* MINIEXPECT_H_ */
diff --git a/miniexpect.pod b/miniexpect.pod
index cd008fc..fe76979 100644
--- a/miniexpect.pod
+++ b/miniexpect.pod
@@ -177,6 +177,18 @@ returned by L<pcre_exec(3)>, and so you can call it
like this:
 
 =back
 
+B<void mexp_set_debug_file (mexp *h, FILE *fp);>
+
+B<FILE *mexp_get_debug_file (mexp *h);>
+
+Set or get the debug file of the handle.  To enable debugging, pass a
+non-C<NULL> file handle, eg. C<stderr>.  To disable debugging, pass
+C<NULL>.  Debugging messages are printed on the file handle.
+
+Note that all output and input gets printed, including passwords.  To
+prevent passwords from being printed, modify your code to call
+C<mexp_printf_password> instead of C<mexp_printf>.
+
 The following fields in the handle do not have methods, but can be
 accessed directly instead:
 
@@ -415,6 +427,8 @@ However we also provide a convenience function:
 
 B<int mexp_printf (mexp_h *h, const char *fs, ...);>
 
+B<int mexp_printf_password (mexp_h *h, const char *fs, ...);>
+
 This returns the number of bytes, if the whole message was written OK.
 If there was an error, -1 is returned and the error is available in
 C<errno>.
@@ -435,6 +449,13 @@ send a command followed by a newline you have to do
something like:
 
  mexp_printf (h, "exit\n");
 
+=item *
+
+C<mexp_printf_password> works identically to C<mexp_printf> except
+that the output is I<not> sent to the debugging file if debugging is
+enabled.  As the name suggests, use this for passwords so that they
+don't appear in debugging output.
+
 =back
 
 B<int mexp_send_interrupt (mexp_h *h);>
-- 
2.13.2

On Wednesday, 11 October 2017 17:23:45 CEST Richard W.M. Jones
wrote:
> +static int
> +mexp_vprintf (mexp_h *h, int password, const char *fs, va_list args)
>  {
> -  va_list args;
>    char *msg;
>    int len;
>    size_t n;
>    ssize_t r;
>    char *p;
>  
> -  va_start (args, fs);
>    len = vasprintf (&msg, fs, args);
> -  va_end (args);
Due to the nature of va_list (whose implementation greatly differs per
architecture -- from a pointer, to a list of pointers, to structs, etc),
I think that here you need to copy the va_list using va_copy(), and
pass the copy to vasprintf.

I cannot find a better reference, but
https://stackoverflow.com/questions/3369588/pass-va-list-or-pointer-to-va-list
has a piece of advice on that.

-- 
Pino Toscano

On Wednesday, 11 October 2017 17:23:44 CEST Richard W.M. Jones
wrote:
> +#if DEBUG
> +/* Print escaped buffer to fp. */
> +static void
> +debug_buffer (FILE *fp, char *buf)
'buf' is read-only here, so it ought to be 'const'.

-- 
Pino Toscano

On Wednesday, 11 October 2017 17:23:43 CEST Richard W.M. Jones
wrote:
> Currently you can only turn on miniexpect debugging by recompiling.
> These two patches make it configurable at runtime, and also improve
> the usefulness of the output.
Mostly LGTM, with the two fixes noted.

-- 
Pino Toscano