Re: [PATCH] shell32: print the correct GUIDs in the shellpath tests.

[Juan Lang]

Hi Reece, I'm not nak'ing your patch.  I just wanted to point out a simpler way:

-static const char *printGUID(const GUID *guid)
+static char *printGUID(const GUID *guid)
 {
-    static char guidSTR[39];
+    char *guidSTR = HeapAlloc(GetProcessHeap(), 0, 39);

Rather than allocating the necessary buffer, you can pass a pointer to
a buffer to printGUID, and have printGUID return that buffer.  Have
the caller declare two GUID buffers, and pass one for the expected
value and the other for the received value.

Cheers, and thanks for looking into the failing tests,
--Juan

[PATCH] shell32: print the correct GUIDs in the shellpath tests.

[Maarten Lankhorst]

Hi Juan,

2008/9/28, Juan Lang :
> Hi Reece, I'm not nak'ing your patch.  I just wanted to point out a simpler
> way:
>
> -static const char *printGUID(const GUID *guid)
> +static char *printGUID(const GUID *guid)
>  {
> - static char guidSTR[39];
> - char *guidSTR = HeapAlloc(GetProcessHeap(), 0, 39);
>
> Rather than allocating the necessary buffer, you can pass a pointer to
> a buffer to printGUID, and have printGUID return that buffer.  Have
> the caller declare two GUID buffers, and pass one for the expected
> value and the other for the received value.

This will cause a valgrind warning, so I don't think it should be a
recommended fix.

Cheers,
Maarten.

[PATCH] shell32: print the correct GUIDs in the shellpath tests.

[Reece Dunn]

2008/9/28 Maarten Lankhorst :
> Hi Juan,
>
> 2008/9/28, Juan Lang :
> > Hi Reece, I'm not nak'ing your patch.  I just wanted to point out a simpler
> > way:
> >
> > -static const char *printGUID(const GUID *guid)
> > +static char *printGUID(const GUID *guid)
> >  {
> > - static char guidSTR[39];
> > - char *guidSTR = HeapAlloc(GetProcessHeap(), 0, 39);
> >
> > Rather than allocating the necessary buffer, you can pass a pointer to
> > a buffer to printGUID, and have printGUID return that buffer.  Have
> > the caller declare two GUID buffers, and pass one for the expected
> > value and the other for the received value.
>
> This will cause a valgrind warning, so I don't think it should be a
> recommended fix.

Do you mean my patch, or Juan's proposed version?

- Reece

[PATCH] shell32: print the correct GUIDs in the shellpath tests.

[Maarten Lankhorst]

Juan's proposed  version will cause a memory leak / valgrind warning.

2008/9/28, Reece Dunn :
> 2008/9/28 Maarten Lankhorst :
> > Hi Juan,
> >
> > 2008/9/28, Juan Lang :
> > > Hi Reece, I'm not nak'ing your patch.  I just wanted to point out a
> > > simpler
> > > way:
> > >
> > > -static const char *printGUID(const GUID *guid)
> > > +static char *printGUID(const GUID *guid)
> > >  {
> > > - static char guidSTR[39];
> > > - char *guidSTR = HeapAlloc(GetProcessHeap(), 0, 39);
> > >
> > > Rather than allocating the necessary buffer, you can pass a pointer to
> > > a buffer to printGUID, and have printGUID return that buffer.  Have
> > > the caller declare two GUID buffers, and pass one for the expected
> > > value and the other for the received value.
> >
> > This will cause a valgrind warning, so I don't think it should be a
> > recommended fix.
>
> Do you mean my patch, or Juan's proposed version?
> - Reece

[PATCH] shell32: print the correct GUIDs in the shellpath tests.

[Juan Lang]

> Juan's proposed  version will cause a memory leak / valgrind warning.

Not at all.  I'm proposing stack pointers, not HeapAlloc'ed versions.
See for example how I fixed this for the SIP tests:
http://www.winehq.org/pipermail/wine-cvs/2008-July/045766.html
--Juan