Re: [PATCH v3 1/4] QGA VSS: Add wrapper to send log to debugger and stderr

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 7/7/23 16:24, Konstantin Kostiuk wrote:
> 
> 
> On Fri, Jul 7, 2023 at 1:33 PM Philippe Mathieu-Daudé <philmd@linaro.org 
> <mailto:philmd@linaro.org>> wrote:
> 
>     On 7/7/23 11:22, Konstantin Kostiuk wrote:
>      > Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com
>     <mailto:kkostiuk@redhat.com>>
>      > ---
>      >   qga/vss-win32/meson.build   |  2 +-
>      >   qga/vss-win32/vss-debug.cpp | 31 +++++++++++++++++++++++++++++++
>      >   qga/vss-win32/vss-debug.h   | 25 +++++++++++++++++++++++++
>      >   3 files changed, 57 insertions(+), 1 deletion(-)
>      >   create mode 100644 qga/vss-win32/vss-debug.cpp
>      >   create mode 100644 qga/vss-win32/vss-debug.h
> 
> 
>      > diff --git a/qga/vss-win32/vss-debug.cpp
>     b/qga/vss-win32/vss-debug.cpp
>      > new file mode 100644
>      > index 0000000000..5d6f37944b
>      > --- /dev/null
>      > +++ b/qga/vss-win32/vss-debug.cpp
>      > @@ -0,0 +1,31 @@
>      > +/*
>      > + * QEMU Guest Agent VSS debug declarations
>      > + *
>      > + * Copyright (C) 2023 Red Hat Inc
>      > + *
>      > + * Authors:
>      > + *  Konstantin Kostiuk <kkostiuk@redhat.com
>     <mailto:kkostiuk@redhat.com>>
>      > + *
>      > + * This work is licensed under the terms of the GNU GPL, version
>     2 or later.
>      > + * See the COPYING file in the top-level directory.
>      > + */
>      > +
>      > +#include "qemu/osdep.h"
>      > +#include "vss-debug.h"
>      > +#include "vss-common.h"
>      > +
>      > +void qga_debug_internal(const char *funcname, const char *fmt,
>     ...)  {
>      > +    char user_sting[512] = {0};
> 
>     "user_string"
> 
>      > +    char full_string[640] = {0};
>      > +
>      > +    va_list args;
>      > +    va_start(args, fmt);
>      > +    vsnprintf(user_sting, 512, fmt, args);
> 
>     ARRAY_SIZE(user_string)
> 
> 
> I have a problem with ARRAY_SIZE define.
> 
> ../qga/vss-win32/vss-debug.cpp: In function 'void 
> qga_debug_internal(const char*, const char*, ...)':
> /src/include/qemu/compiler.h:70:12: error: types may not be defined in 
> 'sizeof' expressions
>     70 |     struct { \
>        |            ^
> /src/include/qemu/compiler.h:78:43: note: in expansion of macro 
> 'QEMU_BUILD_BUG_ON_STRUCT'
>     78 | #define QEMU_BUILD_BUG_ON_ZERO(x) 
> (sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)) - \
>        |                                           ^~~~~~~~~~~~~~~~~~~~~~~~
> /src/include/qemu/osdep.h:474:24: note: in expansion of macro 
> 'QEMU_BUILD_BUG_ON_ZERO'
>    474 |                        QEMU_BUILD_BUG_ON_ZERO(!QEMU_IS_ARRAY(x)))
>        |                        ^~~~~~~~~~~~~~~~~~~~~~
> ../qga/vss-win32/vss-debug.cpp:23:32: note: in expansion of macro 
> 'ARRAY_SIZE'
>     23 |     if (vsnprintf(user_string, ARRAY_SIZE(user_string), fmt, 
> args) <= 0) {
>        |                                ^~~~~~~~~~
> /src/include/qemu/osdep.h:470:57: error: expected primary-expression 
> before 'typeof'
>    470 | #define QEMU_IS_ARRAY(x) 
> (!__builtin_types_compatible_p(typeof(x), \
>        |                                                         ^~~~~~
> /src/include/qemu/compiler.h:71:14: note: in definition of macro 
> 'QEMU_BUILD_BUG_ON_STRUCT'
>     71 |         int:(x) ? -1 : 1; \
>        |              ^
> /src/include/qemu/osdep.h:474:24: note: in expansion of macro 
> 'QEMU_BUILD_BUG_ON_ZERO'
>    474 |                        QEMU_BUILD_BUG_ON_ZERO(!QEMU_IS_ARRAY(x)))
>        |                        ^~~~~~~~~~~~~~~~~~~~~~
> /src/include/qemu/osdep.h:474:48: note: in expansion of macro 
> 'QEMU_IS_ARRAY'
>    474 |                        QEMU_BUILD_BUG_ON_ZERO(!QEMU_IS_ARRAY(x)))
> 
> Can you help with this?

Eh no clue, maybe not C++ compliant?

Fall back to sizeof() ¯\_(ツ)_/¯

> 
> 
>      > +    va_end(args);
>      > +
>      > +    snprintf(full_string, 640, QGA_PROVIDER_NAME "[%lu]: %s %s\n",
>      > +             GetCurrentThreadId(), funcname, user_sting);
> 
>     if (snprintf() <= 0) { return; }
> 
>      > +
>      > +    OutputDebugString(full_string);
>      > +    fprintf(stderr, "%s", full_string);
> 
>     Format unused:
> 
>     fputs(full_string, stderr);
> 
>      > +}
> 
>     With the changes:
>     Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org
>     <mailto:philmd@linaro.org>>
>