diff mbox series

[1/2] hw/display/vmware_vga: only show debug output if DEBUG enabled

Message ID 20220104091135.61226-2-carwynellis@gmail.com (mailing list archive)
State New, archived
Headers show
Series hw/display/vmware_vga: supress debug output and fix | expand

Commit Message

Carwyn Ellis Jan. 4, 2022, 9:11 a.m. UTC 
Debug output was always being sent to STDERR. This has been replaced by
a define that will only show this output when DEBUG is set to true.

Signed-off-by: Carwyn Ellis <carwynellis@gmail.com>
---
 hw/display/vmware_vga.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

Comments

Laurent Vivier Jan. 4, 2022, 9:18 a.m. UTC | #1 
Le 04/01/2022 à 10:11, Carwyn Ellis a écrit :
> Debug output was always being sent to STDERR. This has been replaced by
> a define that will only show this output when DEBUG is set to true.
> 
> Signed-off-by: Carwyn Ellis <carwynellis@gmail.com>
> ---
>   hw/display/vmware_vga.c | 26 ++++++++++++++++----------
>   1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index e2969a6c81..8080e085d1 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -43,6 +43,12 @@
>   
>   /* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
>   
> +#ifdef DEBUG
> +#define VMWARE_VGA_DEBUG(...) { (void) fprintf(stdout, __VA_ARGS__); }
> +#else
> +#define VMWARE_VGA_DEBUG(...) ((void) 0)
> +#endif
> +

Could you replace this macro by adding some trace-events instead.

See https://qemu-project.gitlab.io/qemu/devel/tracing.html#using-trace-events

Thanks,
Laurent

>   struct vmsvga_state_s {
>       VGACommonState vga;
>   
> @@ -297,45 +303,45 @@ static inline bool vmsvga_verify_rect(DisplaySurface *surface,
>                                         int x, int y, int w, int h)
>   {
>       if (x < 0) {
> -        fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
> +        VMWARE_VGA_DEBUG("%s: x was < 0 (%d)\n", name, x);
>           return false;
>       }
>       if (x > SVGA_MAX_WIDTH) {
> -        fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
> +        VMWARE_VGA_DEBUG("%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
>           return false;
>       }
>       if (w < 0) {
> -        fprintf(stderr, "%s: w was < 0 (%d)\n", name, w);
> +        VMWARE_VGA_DEBUG("%s: w was < 0 (%d)\n", name, w);
>           return false;
>       }
>       if (w > SVGA_MAX_WIDTH) {
> -        fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
> +        VMWARE_VGA_DEBUG("%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
>           return false;
>       }
>       if (x + w > surface_width(surface)) {
> -        fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n",
> +        VMWARE_VGA_DEBUG("%s: width was > %d (x: %d, w: %d)\n",
>                   name, surface_width(surface), x, w);
>           return false;
>       }
>   
>       if (y < 0) {
> -        fprintf(stderr, "%s: y was < 0 (%d)\n", name, y);
> +        VMWARE_VGA_DEBUG("%s: y was < 0 (%d)\n", name, y);
>           return false;
>       }
>       if (y > SVGA_MAX_HEIGHT) {
> -        fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
> +        VMWARE_VGA_DEBUG("%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
>           return false;
>       }
>       if (h < 0) {
> -        fprintf(stderr, "%s: h was < 0 (%d)\n", name, h);
> +        VMWARE_VGA_DEBUG("%s: h was < 0 (%d)\n", name, h);
>           return false;
>       }
>       if (h > SVGA_MAX_HEIGHT) {
> -        fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
> +        VMWARE_VGA_DEBUG("%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
>           return false;
>       }
>       if (y + h > surface_height(surface)) {
> -        fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n",
> +        VMWARE_VGA_DEBUG("%s: update height > %d (y: %d, h: %d)\n",
>                   name, surface_height(surface), y, h);
>           return false;
>       }
Carwyn Ellis Jan. 4, 2022, 9:20 a.m. UTC | #2 
Hey,

Thanks for getting back to me.

Yeah will take a look and update when I have a mo.

Cheers
Carwyn

> On 4 Jan 2022, at 09:18, Laurent Vivier <laurent@vivier.eu> wrote:
> 
> Le 04/01/2022 à 10:11, Carwyn Ellis a écrit :
>> Debug output was always being sent to STDERR. This has been replaced by
>> a define that will only show this output when DEBUG is set to true.
>> Signed-off-by: Carwyn Ellis <carwynellis@gmail.com>
>> ---
>>  hw/display/vmware_vga.c | 26 ++++++++++++++++----------
>>  1 file changed, 16 insertions(+), 10 deletions(-)
>> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
>> index e2969a6c81..8080e085d1 100644
>> --- a/hw/display/vmware_vga.c
>> +++ b/hw/display/vmware_vga.c
>> @@ -43,6 +43,12 @@
>>    /* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
>>  +#ifdef DEBUG
>> +#define VMWARE_VGA_DEBUG(...) { (void) fprintf(stdout, __VA_ARGS__); }
>> +#else
>> +#define VMWARE_VGA_DEBUG(...) ((void) 0)
>> +#endif
>> +
> 
> Could you replace this macro by adding some trace-events instead.
> 
> See https://qemu-project.gitlab.io/qemu/devel/tracing.html#using-trace-events
> 
> Thanks,
> Laurent
> 
>>  struct vmsvga_state_s {
>>      VGACommonState vga;
>>  @@ -297,45 +303,45 @@ static inline bool vmsvga_verify_rect(DisplaySurface *surface,
>>                                        int x, int y, int w, int h)
>>  {
>>      if (x < 0) {
>> -        fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
>> +        VMWARE_VGA_DEBUG("%s: x was < 0 (%d)\n", name, x);
>>          return false;
>>      }
>>      if (x > SVGA_MAX_WIDTH) {
>> -        fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
>> +        VMWARE_VGA_DEBUG("%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
>>          return false;
>>      }
>>      if (w < 0) {
>> -        fprintf(stderr, "%s: w was < 0 (%d)\n", name, w);
>> +        VMWARE_VGA_DEBUG("%s: w was < 0 (%d)\n", name, w);
>>          return false;
>>      }
>>      if (w > SVGA_MAX_WIDTH) {
>> -        fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
>> +        VMWARE_VGA_DEBUG("%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
>>          return false;
>>      }
>>      if (x + w > surface_width(surface)) {
>> -        fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n",
>> +        VMWARE_VGA_DEBUG("%s: width was > %d (x: %d, w: %d)\n",
>>                  name, surface_width(surface), x, w);
>>          return false;
>>      }
>>        if (y < 0) {
>> -        fprintf(stderr, "%s: y was < 0 (%d)\n", name, y);
>> +        VMWARE_VGA_DEBUG("%s: y was < 0 (%d)\n", name, y);
>>          return false;
>>      }
>>      if (y > SVGA_MAX_HEIGHT) {
>> -        fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
>> +        VMWARE_VGA_DEBUG("%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
>>          return false;
>>      }
>>      if (h < 0) {
>> -        fprintf(stderr, "%s: h was < 0 (%d)\n", name, h);
>> +        VMWARE_VGA_DEBUG("%s: h was < 0 (%d)\n", name, h);
>>          return false;
>>      }
>>      if (h > SVGA_MAX_HEIGHT) {
>> -        fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
>> +        VMWARE_VGA_DEBUG("%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
>>          return false;
>>      }
>>      if (y + h > surface_height(surface)) {
>> -        fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n",
>> +        VMWARE_VGA_DEBUG("%s: update height > %d (y: %d, h: %d)\n",
>>                  name, surface_height(surface), y, h);
>>          return false;
>>      }
>
Laurent Vivier Jan. 4, 2022, 9:27 a.m. UTC | #3 
Le 04/01/2022 à 10:20, Carwyn Ellis a écrit :
> Hey,
> 
> Thanks for getting back to me.
> 
> Yeah will take a look and update when I have a mo.

It's really easy to do, see below for an example:

...
>>>   @@ -297,45 +303,45 @@ static inline bool vmsvga_verify_rect(DisplaySurface *surface,
>>>                                         int x, int y, int w, int h)
>>>   {
>>>       if (x < 0) {
>>> -        fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
>>> +        VMWARE_VGA_DEBUG("%s: x was < 0 (%d)\n", name, x);

replace it by:

     trace_vmsvga_verify_rect_check_neg(name, x);

and in hw/display/trace-events you add:

     vmsvga_verify_rect_check_neg(const char *name, int x) "%s: x was < 0 (%d)"

Thanks,
Laurent
Carwyn Ellis Jan. 4, 2022, 9:28 a.m. UTC | #4 
Ok cool.

Thanks for the info!

> On 4 Jan 2022, at 09:27, Laurent Vivier <laurent@vivier.eu> wrote:
> 
> Le 04/01/2022 à 10:20, Carwyn Ellis a écrit :
>> Hey,
>> Thanks for getting back to me.
>> Yeah will take a look and update when I have a mo.
> 
> It's really easy to do, see below for an example:
> 
> ...
>>>>  @@ -297,45 +303,45 @@ static inline bool vmsvga_verify_rect(DisplaySurface *surface,
>>>>                                        int x, int y, int w, int h)
>>>>  {
>>>>      if (x < 0) {
>>>> -        fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
>>>> +        VMWARE_VGA_DEBUG("%s: x was < 0 (%d)\n", name, x);
> 
> replace it by:
> 
>    trace_vmsvga_verify_rect_check_neg(name, x);
> 
> and in hw/display/trace-events you add:
> 
>    vmsvga_verify_rect_check_neg(const char *name, int x) "%s: x was < 0 (%d)"
> 
> Thanks,
> Laurent
diff mbox series

Patch

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index e2969a6c81..8080e085d1 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -43,6 +43,12 @@ 
 
 /* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
 
+#ifdef DEBUG
+#define VMWARE_VGA_DEBUG(...) { (void) fprintf(stdout, __VA_ARGS__); }
+#else
+#define VMWARE_VGA_DEBUG(...) ((void) 0)
+#endif
+
 struct vmsvga_state_s {
     VGACommonState vga;
 
@@ -297,45 +303,45 @@  static inline bool vmsvga_verify_rect(DisplaySurface *surface,
                                       int x, int y, int w, int h)
 {
     if (x < 0) {
-        fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
+        VMWARE_VGA_DEBUG("%s: x was < 0 (%d)\n", name, x);
         return false;
     }
     if (x > SVGA_MAX_WIDTH) {
-        fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
+        VMWARE_VGA_DEBUG("%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
         return false;
     }
     if (w < 0) {
-        fprintf(stderr, "%s: w was < 0 (%d)\n", name, w);
+        VMWARE_VGA_DEBUG("%s: w was < 0 (%d)\n", name, w);
         return false;
     }
     if (w > SVGA_MAX_WIDTH) {
-        fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
+        VMWARE_VGA_DEBUG("%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
         return false;
     }
     if (x + w > surface_width(surface)) {
-        fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n",
+        VMWARE_VGA_DEBUG("%s: width was > %d (x: %d, w: %d)\n",
                 name, surface_width(surface), x, w);
         return false;
     }
 
     if (y < 0) {
-        fprintf(stderr, "%s: y was < 0 (%d)\n", name, y);
+        VMWARE_VGA_DEBUG("%s: y was < 0 (%d)\n", name, y);
         return false;
     }
     if (y > SVGA_MAX_HEIGHT) {
-        fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
+        VMWARE_VGA_DEBUG("%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
         return false;
     }
     if (h < 0) {
-        fprintf(stderr, "%s: h was < 0 (%d)\n", name, h);
+        VMWARE_VGA_DEBUG("%s: h was < 0 (%d)\n", name, h);
         return false;
     }
     if (h > SVGA_MAX_HEIGHT) {
-        fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
+        VMWARE_VGA_DEBUG("%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
         return false;
     }
     if (y + h > surface_height(surface)) {
-        fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n",
+        VMWARE_VGA_DEBUG("%s: update height > %d (y: %d, h: %d)\n",
                 name, surface_height(surface), y, h);
         return false;
     }