Re: tests: Avoid duplicating debugstr_guid()

[Jacek Caban]

Hi Frédéric,

On 01/15/14 23:53, Frédéric Delanoy wrote:
> diff --git a/include/wine/test.h b/include/wine/test.h
> index 373fe76..d7ce8c1 100644
> --- a/include/wine/test.h
> +++ b/include/wine/test.h
> @@ -65,6 +65,7 @@ extern void winetest_add_failures( LONG new_failures );
>  extern void winetest_wait_child_process( HANDLE process );
>
> extern const char *wine_dbgstr_wn( const WCHAR *str, int n );
> +extern const char *debugstr_guid( const GUID *guid );

You probably want to follow existing test.h convention and call it
wine_dbgstr_guid. This has a bonus advantage that you may split the
patch for function introduction and per-dll replace.
> static inline const char *wine_dbgstr_w( const WCHAR *s ) { return wine_dbgstr_wn( s, -1 ); }
>
> /* strcmpW is available for tests compiled under Wine, but not in standalone
> @@ -523,6 +524,21 @@ const char *wine_dbgstr_wn( const WCHAR *str, int n )
>      return res;
>  }
>
> +const char *debugstr_guid( const GUID *guid )
> +{
> - static char buf[50];
> -
> - if (!guid)
> -
>    return "(null)";
> -
> - sprintf(buf, "{%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}",
> -
>        guid->Data1, guid->Data2, guid->Data3, guid->Data4[0],
> -
>        guid->Data4[1], guid->Data4[2], guid->Data4[3], guid->Data4[4],
> -
>        guid->Data4[5], guid->Data4[6], guid->Data4[7]);
> -
> - return buf;
>
> +}

It would be a good idea to use get_temp_buffer, like wine_dbgstr_wn,
instead of static variable.

Thanks,
Jacek

tests: Avoid duplicating debugstr_guid()

[Frédéric Delanoy]

On Thu, Jan 16, 2014 at 1:02 PM, Jacek Caban  wrote:
> Hi Frédéric,
>
> On 01/15/14 23:53, Frédéric Delanoy wrote:
> > diff --git a/include/wine/test.h b/include/wine/test.h
> > index 373fe76..d7ce8c1 100644
> > --- a/include/wine/test.h
> > +++ b/include/wine/test.h
> > @@ -65,6 +65,7 @@ extern void winetest_add_failures( LONG new_failures );
> >  extern void winetest_wait_child_process( HANDLE process );
> >
> > extern const char *wine_dbgstr_wn( const WCHAR *str, int n );
> > +extern const char *debugstr_guid( const GUID *guid );
>
> You probably want to follow existing test.h convention and call it
> wine_dbgstr_guid. This has a bonus advantage that you may split the
> patch for function introduction and per-dll replace.

I just didn't rename it to keep the patch minimal by avoiding renaming
every call but OK.
Do you think I should split that patch in order to have 1! patch per
DLL? Or is that overkill?
> Thanks,
> Jacek

tests: Avoid duplicating debugstr_guid()

[Jacek Caban]

On 01/16/14 18:49, Frédéric Delanoy wrote:
> On Thu, Jan 16, 2014 at 1:02 PM, Jacek Caban  wrote:
> > Hi Frédéric,
> >
> > On 01/15/14 23:53, Frédéric Delanoy wrote:
> > > diff --git a/include/wine/test.h b/include/wine/test.h
> > > index 373fe76..d7ce8c1 100644
> > > --- a/include/wine/test.h
> > > +++ b/include/wine/test.h
> > > @@ -65,6 +65,7 @@ extern void winetest_add_failures( LONG new_failures );
> > >  extern void winetest_wait_child_process( HANDLE process );
> > >
> > > extern const char *wine_dbgstr_wn( const WCHAR *str, int n );
> > > +extern const char *debugstr_guid( const GUID *guid );
> >
> > You probably want to follow existing test.h convention and call it
> > wine_dbgstr_guid. This has a bonus advantage that you may split the
> > patch for function introduction and per-dll replace.
>
> I just didn't rename it to keep the patch minimal by avoiding renaming
> every call but OK.
> Do you think I should split that patch in order to have 1! patch per
> DLL? Or is that overkill?

Sounds good to me.

Cheers,
Jacek