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

[Konstantin Kostiuk <kkostiuk@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2491 bytes --]

On Thu, Jul 6, 2023 at 1:01 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 6/7/23 09:54, Konstantin Kostiuk wrote:
> > Hi Philippe,
> >
> > On Wed, Jul 5, 2023 at 11:35 PM Philippe Mathieu-Daudé
> > <philmd@linaro.org <mailto:philmd@linaro.org>> wrote:
> >
> >     Hi Konstantin,
> >
> >     On 5/7/23 16:12, Konstantin Kostiuk wrote:
> >      > Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com
> >     <mailto:kkostiuk@redhat.com>>
> >      > ---
> >      >   qga/vss-win32/vss-debug.h | 31 +++++++++++++++++++++++++++++++
> >      >   1 file changed, 31 insertions(+)
> >      >   create mode 100644 qga/vss-win32/vss-debug.h
> >
> >
> >      > +#define PRINT_DEBUG(fmt, ...) {
> >                   \
> >      > +    char user_sting[512] = { 0 };
> >                   \
> >      > +    char full_string[640] = { 0 };
> >                    \
> >      > +    snprintf(user_sting, 512, fmt, ## __VA_ARGS__);
> >                   \
> >      > +    snprintf(full_string, 640, QGA_PROVIDER_NAME"[%lu]: %s
> >     %s\n",             \
> >      > +        GetCurrentThreadId(), __func__, user_sting);
> >                    \
> >      > +    OutputDebugString(full_string);
> >                   \
> >      > +    fprintf(stderr, "%s", full_string);
> >                   \
> >      > +}
> >
> >     Why not simply use a plain function?
> >
> >
> > I am not sure what you mean.
> >
> > If you mean to call OutputDebugString directly, then we need to
> > concatenate the proper string
> > for each call.
> > If you mean convert PRINT_DEBUG to function, then we can't use the
> > __func__ macro in
> > PRINT_DEBUG to get the real function name. We can convert PRINT_DEBUG to
> > function
> > and a new macro that will call PRINT_DEBUG and pass a proper value of
> > __fucn__.
> >
> > What solution is better there?
>
>    void qga_debug(const char *funcname, const char *fmt, ...);
>
> and g_strdup_vprintf()?
>


I agree about the qga_debug function.
About g_strdup_vprintf I am not sure. Currently, VSS uses only one function
from GLib (g_assertion_message_expr).
In the past, we had problems with GLib due to mistakes in Windows
implementation and we thinking about
removing GLib from VSS at all. As VSS.DLL is also invoked directly from
Windows, we should use standard
C++ types in public functions. We can use GLib types/functions for the
internal part of DLL, but I think it will
be confusing.

[-- Attachment #2: Type: text/html, Size: 4138 bytes --]