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
> >
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
