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

2011-04-08 Thread Hans de Goede

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

Both derived from Patrick Powell's original snprintf.c, but with things
like floating point support.

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 000..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:
+ 

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

2011-04-07 Thread Alon Levy
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 000..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);
+