vsnprintf: Expand \n to \r\n for console output

xenconsoled doesn't automatically convert \n into \r\n, which causes test
output to appear like this in some terminals:

  [root@host ~]# xl create -c tests/selftest/test-pv64-selftest.cfg
  Parsing config from tests/selftest/test-pv64-selftest.cfg
  --- Xen Test Framework ---
                            Environment: PV 64bit (Long mode 4 levels)
                                                                      XTF Selftests

There are a number of ways to do this, but by far the most efficient way is to
have vsnprintf() expand \n's in the output buffer.

This however is non-standard behaviour for vsnprintf().  Rename it to
vsnprintf_internal() and take extra flags, and have vprintk() use the new
LF_TO_CRLF control flag.

Inside vsnprintf_internal(), rearrange the non-format and %c logic to share
the expansion logic, as well as extending the logic to fmt_string().

Extend the selftests to confirm correct behaviour in both modes, for all ways
of being able to pass newline characters into a format operation.

Reported-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/common/console.c b/common/console.c
index 0724fc9f4e2cc81aad6f5c6ea1806a21ca79ba73..b3e89473a7b43266b41b72dff45d7865aa199db9 100644 (file)
--- a/common/console.c
+++ b/common/console.c
@@ -122,7 +122,7 @@ void vprintk(const char *fmt, va_list args)
     unsigned int i;
     int rc;
 
-    rc = vsnprintf(buf, sizeof(buf), fmt, args);
+    rc = vsnprintf_internal(buf, sizeof(buf), fmt, args, LF_TO_CRLF);
 
     if ( rc > (int)sizeof(buf) )
         panic("vprintk() buffer overflow\n");

diff --git a/common/libc/vsnprintf.c b/common/libc/vsnprintf.c
index a49fd308b7046346e00a81a6ff71ab2cf339b5ee..c907d42ba0f52815f77fcae96062c0eda0c23960 100644 (file)
--- a/common/libc/vsnprintf.c
+++ b/common/libc/vsnprintf.c
@@ -47,6 +47,7 @@ static int isdigit(int c)
 /* Conversions */
 #define UPPER     (1u << 5)
 #define SIGNED    (1u << 6)
+/* Careful not to overlap with vsnprintf_internal() flags. */
 
 /* Shorthand for ensuring str moves forwards, but not overruning the buffer. */
 #define PUT(c)                                  \
@@ -185,7 +186,11 @@ char *fmt_string(char *str, char *end, const char *val,
             PUT(' ');
 
     for ( i = 0; i < len; ++i )
+    {
+        if ( (flags & LF_TO_CRLF) && val[i] == '\n' )
+            PUT('\r');
         PUT(val[i]);
+    }
 
     while ( len < width-- )
         PUT(' ');
@@ -268,7 +273,8 @@ static char *pointer(
                       width, precision, flags);
 }
 
-int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
+int vsnprintf_internal(char *buf, size_t size, const char *fmt, va_list args,
+                       unsigned int caller_flags)
 {
     char *str = buf, *end = buf + size;
 
@@ -277,15 +283,15 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
         const char *spec_start = fmt; /* For rewinding on error. */
 
         unsigned long long num;
-        unsigned int base, flags = 0;
+        unsigned int base, flags = caller_flags;
         int width = -1, precision = -1;
-        char length_mod = 'i';
+        char c, length_mod = 'i';
 
         /* Put regular characters into the destination. */
         if ( *fmt != '%' )
         {
-            PUT(*fmt);
-            continue;
+            c = *fmt;
+            goto put_char;
         }
 
  next_flag: /* Process any flags. */
@@ -359,20 +365,21 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
             continue;
 
         case 'c': /* Unsigned char. */
-        {
-            unsigned char c = va_arg(args, int);
+            c = va_arg(args, int);
 
             if ( !(flags & LEFT) )
                 while ( --width > 0 )
                     PUT(' ');
 
+        put_char:
+            if ( (flags & LF_TO_CRLF) && c == '\n' )
+                PUT('\r');
             PUT(c);
 
             while ( --width > 0 )
                 PUT(' ');
 
             continue;
-        }
 
         case 's': /* String. */
             str = fmt_string(str, end, va_arg(args, const char *),

diff --git a/include/xtf/libc.h b/include/xtf/libc.h
index 66f834b474c4b30d0b8597f96cc9610522363810..f24a631f35e8f1a31aa9ce8a8a75528c3b6f9d70 100644 (file)
--- a/include/xtf/libc.h
+++ b/include/xtf/libc.h
@@ -37,8 +37,21 @@ int memcmp(const void *s1, const void *s2, size_t n);
 
 size_t strnlen(const char *str, size_t max);
 
+/*
+ * Internal version of vsnprintf(), taking extra control flags.
+ *
+ * LF_TO_CRLF causes "\n" to be expanded to "\r\n" in the output buffer.
+ */
+#define LF_TO_CRLF (1u << 7)
 int __printf(3, 0)
-    vsnprintf(char *buf, size_t size, const char *fmt, va_list args);
+    vsnprintf_internal(char *buf, size_t size, const char *fmt, va_list args,
+                       unsigned int flags);
+
+static inline int __printf(3, 0)
+    vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
+{
+    return vsnprintf_internal(buf, size, fmt, args, 0);
+}
 
 int __printf(3, 4)
     snprintf(char *buf, size_t size, const char *fmt, ...);

diff --git a/tests/selftest/main.c b/tests/selftest/main.c
index c2f6e727fd3080660a4ec8fb1c404f7ca47a15f3..a5c205bab609491406d8d9fd13593c872e04ca73 100644 (file)
--- a/tests/selftest/main.c
+++ b/tests/selftest/main.c
@@ -340,6 +340,43 @@ static void test_driver_init(void)
         xtf_failure("Fail: xtf_init_grant_table(2) returned %d\n", rc);
 }
 
+static void test_vsnprintf_crlf_one(const char *fmt, ...)
+{
+    va_list args;
+
+    char buf[4];
+    int rc;
+
+    va_start(args, fmt);
+    rc = vsnprintf(buf, sizeof(buf), fmt, args);
+    va_end(args);
+
+    if ( rc != 1 )
+        return xtf_failure("Fail: '%s', expected length 1, got %d\n", fmt, rc);
+    if ( strcmp(buf, "\n") )
+        return xtf_failure("Fail: '%s', expected \"\\n\", got %*ph\n",
+                           fmt, (int)sizeof(buf), buf);
+
+    va_start(args, fmt);
+    rc = vsnprintf_internal(buf, sizeof(buf), fmt, args, LF_TO_CRLF);
+    va_end(args);
+
+    if ( rc != 2 )
+        return xtf_failure("Fail: '%s', expected length 2, got %d\n", fmt, rc);
+    if ( strcmp(buf, "\r\n") )
+        return xtf_failure("Fail: '%s', expected \"\\r\\n\", got %*ph\n",
+                           fmt, (int)sizeof(buf), buf);
+}
+
+static void test_vsnprintf_crlf(void)
+{
+    printk("Test: vsnprintf() with CRLF expansion\n");
+
+    test_vsnprintf_crlf_one("\n");
+    test_vsnprintf_crlf_one("%c", '\n');
+    test_vsnprintf_crlf_one("%s", "\n");
+}
+
 void test_main(void)
 {
     /*
@@ -368,6 +405,7 @@ void test_main(void)
     test_extable_handler();
     test_custom_idte();
     test_driver_init();
+    test_vsnprintf_crlf();
 
     if ( has_xenstore )
         test_xenstore();