[Spice-devel] [PATHCH win-qxl 3/8] miniport: add QXL_IO_LOG
Hans de Goede
hdegoede at redhat.com
Fri Apr 8 02:28:13 PDT 2011
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 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