[Spice-devel] [PATHCH win-qxl 3/8] miniport: add QXL_IO_LOG

Alon Levy alevy at redhat.com
Fri Apr 8 06:10:03 PDT 2011


On Fri, Apr 08, 2011 at 11:28:13AM +0200, Hans de Goede wrote:
> Hmm,
> 
> snprintf.c is missing a copyright header, also implementing our own
> snprintf feels like a serious case of NIH syndrome. Please use an
> existing proven implementation like one of these:
> http://www.jhweiss.de/software/snprintf.html
> http://www.opensource.apple.com/source/lukemftpd/lukemftpd-11/tnftpd/libnetbsd/snprintf.c
> 
ok, thanks, I was looking for something, tried taking the one from glibc but
it was way too macroed to be easy to copy out for me. otoh I'm pretty happy with what
I got, and I do have a test suite it passed (didn't commit it). I'll look at those
links.

> Both derived from Patrick Powell's original snprintf.c, but with things
> like floating point support.
which we don't need right now, but we might later.

> 
> Regards,
> 
> Hans
> 
> 
> On 04/07/2011 06:10 PM, Alon Levy wrote:
> >Until now logging for miniport and display went in different paths, display
> >knew how to use the QXL_IO_LOG port to allow basically seeing the debug
> >messages on the host with the rest of the qemu monitor output. But miniport
> >always went through VideoPortDbg, which made it harder to pretend there is
> >nothing like DebugView/WinDBG and use printf's to debug some stuff.
> >
> >The main reason for this is that snprintf is not provided by the minimal API
> >you can use in the miniport. So to code around this this patch adds a minimal
> >snprintf and vsnprintf implementation, and if guestdebug is not 0 then it uses
> >that.
> >
> >The only missing bit is a semaphore to make sure the driver and miniport don't
> >overwrite one another's logs, but that wasn't seen in practice.
> >---
> >  miniport/minimal_snprintf.c |  150 +++++++++++++++++++++++++++++++++++++++++++
> >  miniport/qxl.c              |   55 +++++++++++++++-
> >  miniport/qxl.h              |    3 +-
> >  miniport/sources            |    1 +
> >  4 files changed, 206 insertions(+), 3 deletions(-)
> >  create mode 100644 miniport/minimal_snprintf.c
> >
> >diff --git a/miniport/minimal_snprintf.c b/miniport/minimal_snprintf.c
> >new file mode 100644
> >index 0000000..133b84e
> >--- /dev/null
> >+++ b/miniport/minimal_snprintf.c
> >@@ -0,0 +1,150 @@
> >+#include "minimal_snprintf.h"
> >+
> >+static char lower_digits[] = "0123456789abcdef";
> >+static char upper_digits[] = "0123456789abcdef";
> >+
> >+char *print_unsigned(char *str, size_t *size, long long val, int base,
> >+                     const char *digits)
> >+{
> >+    int n = 0;
> >+    char *strout;
> >+    long long temp = val;
> >+
> >+    if (*size == 0) {
> >+        return str;
> >+    }
> >+    if (base<  0 || base>  16) {
> >+        base = 10;
> >+    }
> >+    while (temp != 0) {
> >+        temp /= base;
> >+        n++;
> >+    }
> >+    if (n>  0) {
> >+        n--;
> >+    }
> >+    while (n>= (int)*size) {
> >+        val /= base;
> >+        n--;
> >+    }
> >+    strout = str + n + 1;
> >+    while (n>= 0&&  *size) {
> >+        (*size)--;
> >+        *(str + n) = digits[(val % base)];
> >+        val /= base;
> >+        n--;
> >+    }
> >+    return strout;
> >+}
> >+
> >+char *print_signed(char *str, size_t *size, long long val, int base,
> >+                   const char *digits)
> >+{
> >+    if (*size == 0) {
> >+        return str;
> >+    }
> >+    if (val<  0) {
> >+        val = -val;
> >+        *(str++) = '-';
> >+        (*size)--;
> >+    }
> >+    return print_unsigned(str, size, val, base, digits);
> >+}
> >+
> >+char *print_number(char *str, size_t *size, long long val, int base,
> >+                const char *digits, int is_signed)
> >+{
> >+    if (is_signed) {
> >+        return print_signed(str, size, val, base, digits);
> >+    }
> >+    return print_unsigned(str, size, val, base, digits);
> >+}
> >+
> >+char *print_string(char *str, size_t *size, char *arg)
> >+{
> >+    for(; *arg&&  *size; ++arg, ++str, --(*size)) {
> >+        *str = *arg;
> >+    }
> >+    return str;
> >+}
> >+
> >+int minimal_vsnprintf(char *str, size_t size, const char *format, va_list ap)
> >+{
> >+    char *str_arg;
> >+    int signed_arg;
> >+    unsigned unsigned_arg;
> >+    int orig_size = size;
> >+    int is_long;
> >+    int is_signed;
> >+    int base;
> >+    long long val;
> >+
> >+    while (*format&&  size) {
> >+        switch (*format) {
> >+        case '%':
> >+            is_signed = 1;
> >+            is_long = 0;
> >+repeat:
> >+            format++;
> >+            switch (*format) {
> >+            case 'l':
> >+                is_long = 1;
> >+                goto repeat;
> >+                break;
> >+            case 'd':
> >+            case 'x':
> >+            case 'u':
> >+                switch (*format) {
> >+                case 'u':
> >+                    is_signed = 0;
> >+                case 'd':
> >+                    base = 10;
> >+                    break;
> >+                case 'x':
> >+                    is_signed = 0;
> >+                    base = 16;
> >+                    break;
> >+                }
> >+                if (is_long) {
> >+                    if (is_signed) {
> >+                        val = (long long)va_arg(ap, long int);
> >+                    } else {
> >+                        val = (long long)va_arg(ap, long unsigned);
> >+                    }
> >+                } else {
> >+                    if (is_signed) {
> >+                        val = (long long)va_arg(ap, int);
> >+                    } else {
> >+                        val = (long long)va_arg(ap, unsigned);
> >+                    }
> >+                }
> >+                str = print_number(str,&size, val, base, lower_digits, is_signed);
> >+                break;
> >+            case 's':
> >+                str = print_string(str,&size, va_arg(ap, char*));
> >+                break;
> >+            default:
> >+                /* unrecognized type, ignored (this *is* minimal) */
> >+                break;
> >+            }
> >+            format++;
> >+            break;
> >+        default:
> >+            *(str++) = *(format++); size--;
> >+            break;
> >+        }
> >+    }
> >+    *str = '\0';
> >+    return orig_size - size;
> >+}
> >+
> >+int minimal_snprintf(char *str, size_t size, const char *format, ...)
> >+{
> >+    va_list ap;
> >+    int ret;
> >+
> >+    va_start(ap, format);
> >+    ret = minimal_vsnprintf(str, size, format, ap);
> >+    va_end(ap);
> >+    return ret;
> >+}
> >diff --git a/miniport/qxl.c b/miniport/qxl.c
> >index 855b08b..3eb71a9 100644
> >--- a/miniport/qxl.c
> >+++ b/miniport/qxl.c
> >@@ -19,11 +19,15 @@
> >     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> >  */
> >
> >+#include "stddef.h"
> >+#include<stdlib.h>
> >+#include "miniport.h"
> >  #include "os_dep.h"
> >  #include "qxl.h"
> >  #if (WINVER<  0x0501)
> >  #include "wdmhelper.h"
> >  #endif
> >+#include "minimal_snprintf.h"
> >
> >  VP_STATUS FindAdapter(PVOID dev_extension,
> >                        PVOID reserved,
> >@@ -91,6 +95,53 @@ typedef struct QXLExtension {
> >
> >  #define QXL_ALLOC_TAG '_lxq'
> >
> >+static char *g_log_buf = NULL;
> >+static PUCHAR g_log_port = 0;
> >+static UINT32 *g_log_level = NULL;
> >+
> >+#define DBG_LEVEL 0
> >+
> >+#define QXL_MINIPORT_DEBUG_PREFIX "qxlmp: "
> >+
> >+void my_memcpy(char *dest, const char *src, unsigned n)
> >+{
> >+    while (n-->  0) {
> >+        *(dest++) = *(src++);
> >+    }
> >+}
> >+
> >+void DebugPrintV(char *log_buf, PUCHAR log_port, const char *message, const char *func, va_list ap)
> >+{
> >+    int n, n_strlen;
> >+
> >+    if (log_buf&&  log_port&&  g_log_level&&  *g_log_level>  0) {
> >+        /*
> >+         * TODO: use a shared semaphore with display code.
> >+         * In practice this is not a problem, since miniport runs either on ioctls (sync)
> >+         * or before display is brought up or when it is brought down.
> >+         * Also the worst that can happen is overwriting a message (not seen in practice).
> >+         */
> >+        minimal_snprintf(log_buf, QXL_LOG_BUF_SIZE, QXL_MINIPORT_DEBUG_PREFIX);
> >+        minimal_vsnprintf(log_buf + strlen(QXL_MINIPORT_DEBUG_PREFIX),
> >+                   QXL_LOG_BUF_SIZE - strlen(QXL_MINIPORT_DEBUG_PREFIX),
> >+                   message, ap);
> >+        VideoPortWritePortUchar(log_port, 0);
> >+    } else {
> >+        VideoDebugPrint((0, (PCHAR)message, ap));
> >+    }
> >+}
> >+
> >+void DebugPrint(UINT32 level, const char *message, const char *func, ...)
> >+{
> >+    va_list ap;
> >+
> >+    if (level>  (g_log_level ? *g_log_level : DBG_LEVEL)) {
> >+        return;
> >+    }
> >+    va_start(ap, message);
> >+    DebugPrintV(g_log_buf, g_log_port, message, func, ap);
> >+    va_end(ap);
> >+}
> >
> >  ULONG DriverEntry(PVOID context1, PVOID context2)
> >  {
> >@@ -153,7 +204,8 @@ VP_STATUS InitIO(QXLExtension *dev, PVIDEO_ACCESS_RANGE range)
> >
> >      dev->io_base = io_base;
> >      dev->io_port = (PUCHAR)range->RangeStart.LowPart;
> >-
> >+    g_log_port = dev->io_port + QXL_IO_LOG;
> >+    g_log_level =&dev->rom->log_level;
> >      DEBUG_PRINT((0, "%s: OK, io 0x%x size %lu\n", __FUNCTION__,
> >                   (ULONG)range->RangeStart.LowPart, range->RangeLength));
> >
> >@@ -259,6 +311,7 @@ VP_STATUS InitRam(QXLExtension *dev, PVIDEO_ACCESS_RANGE range)
> >      dev->ram_start = ram;
> >      dev->ram_header = ram_header;
> >      dev->ram_size = range->RangeLength;
> >+    g_log_buf = dev->ram_header->log_buf;
> >      DEBUG_PRINT((0, "%s OK: ram 0x%lx size %lu\n",
> >                   __FUNCTION__, (ULONG)range->RangeStart.QuadPart, range->RangeLength));
> >      return NO_ERROR;
> >diff --git a/miniport/qxl.h b/miniport/qxl.h
> >index a65d590..c7df4b1 100644
> >--- a/miniport/qxl.h
> >+++ b/miniport/qxl.h
> >@@ -35,8 +35,7 @@ enum {
> >  #define PAGE_SIZE (1<<  PAGE_SHIFT)
> >
> >  #if DBG
> >-#define DEBUG_PRINT(arg) VideoDebugPrint(arg)
> >+#define DEBUG_PRINT(arg) DebugPrint arg
> >  #else
> >  #define DEBUG_PRINT(arg)
> >  #endif
> >-
> >diff --git a/miniport/sources b/miniport/sources
> >index 15b1c76..9391cbf 100644
> >--- a/miniport/sources
> >+++ b/miniport/sources
> >@@ -22,6 +22,7 @@ MSC_WARNING_LEVEL=$(MSC_WARNING_LEVEL) /WX
> >  INCLUDES=$(SPICE_COMMON_DIR); ..\include
> >
> >  SOURCES=qxl.c      \
> >+	minimal_snprintf.c \
> >  		wdmhelper.c \
> >          qxl.rc
> >


More information about the Spice-devel mailing list