Re: [2/2] secur32/tests: Fix GetUserNameEx() tests (try 2)

[Paul Vriens]

Ge van Geldorp wrote:
> Try 2: use win_skip() instead of skip()
>
> Changelog:
>   secur32/tests: Fix GetUserNameEx() tests
>
> ---
>  dlls/secur32/tests/secur32.c |   44
> +++++++++++++++++++++---------------------
>  1 files changed, 22 insertions(+), 22 deletions(-)
>
> ------------------------------------------------------------------------

Hi Ge,

Why did you remove:

-    if (0) /* Crashes on Windows */
-        rc = pGetUserNameExW(NameSamCompatible, NULL, NULL);
-

The point of these additions was to show/document that Windows crashes 
when called like this.

-- 
Cheers,

Paul.

[2/2] secur32/tests: Fix GetUserNameEx() tests (try 2)

[Ge van Geldorp]

Hi Paul,
> From: Paul Vriens
>
> Why did you remove:
>
> -    if (0) /* Crashes on Windows */
> -        rc = pGetUserNameExW(NameSamCompatible, NULL, NULL);
> -
>
> The point of these additions was to show/document that Windows crashes 
> when called like this.

Because Alexandre told me for an earlier patch to remove tests that crash on
Windows: http://www.winehq.org/pipermail/wine-devel/2009-January/072010.html

Ge.

[2/2] secur32/tests: Fix GetUserNameEx() tests (try 2)

[Paul Vriens]

Ge van Geldorp wrote:
> Hi Paul,
> > From: Paul Vriens
> >
> > Why did you remove:
> >
> > -    if (0) /* Crashes on Windows */
> > -        rc = pGetUserNameExW(NameSamCompatible, NULL, NULL);
> > -
> >
> > The point of these additions was to show/document that Windows crashes 
> > when called like this.
>
> Because Alexandre told me for an earlier patch to remove tests that crash on
> Windows: http://www.winehq.org/pipermail/wine-devel/2009-January/072010.html
>
> Ge.

But that refers to a function that is said to be broken on Windows.

I've added these to the tests to show that Windows crashes. This is thus 
a test (although not run) for behavior. It just means that Windows 
doesn't check the input parameters thoroughly and so we shouldn't 
either. It was trigerred by Coverity that reported a possible 
dereference on a NULL parameter.

-- 
Cheers,

Paul.

[2/2] secur32/tests: Fix GetUserNameEx() tests (try 2)

[Ge van Geldorp]

Hi Paul,
> From: Paul Vriens [mailto:paul.vriens.wine(a)gmail.com]
>
> I've added these to the tests to show that Windows crashes. This is thus 
> a test (although not run) for behavior. It just means that Windows 
> doesn't check the input parameters thoroughly and so we shouldn't 
> either. It was trigerred by Coverity that reported a possible 
> dereference on a NULL parameter.

Sorry, I hadn't noticed that you changed something between my initial
version of these tests and my fixes this weekend. I'll put them back. I
still don't see the difference between this and the earlier case, but I've
given up on trying to find any consistency in Wine submissions anyway.

Ge.

[2/2] secur32/tests: Fix GetUserNameEx() tests (try 2)

[Kai Blin]

On Tuesday 14 April 2009 12:39:51 Ge van Geldorp wrote:

Hi Ge,
> > Why did you remove:
> >
> > -    if (0) /* Crashes on Windows */
> > -        rc = pGetUserNameExW(NameSamCompatible, NULL, NULL);
> > -
> >
> > The point of these additions was to show/document that Windows crashes
> > when called like this.
>
> Because Alexandre told me for an earlier patch to remove tests that crash
> on Windows:
> http://www.winehq.org/pipermail/wine-devel/2009-January/072010.html

What Paul meant is that as this test is behind an if(0), it's not going to be 
run anyway. So all it does is to serve as documentation of Windows behaviour.
Arguably, this could just be a comment, ideally in the implementation of 
GetUserNameExW, but in general it's nice to have this sort of information.

Cheers,
Kai

-- 
Kai Blin
WorldForge developer  http://www.worldforge.org/
Wine developer        http://wiki.winehq.org/KaiBlin
Samba team member     http://www.samba.org/samba/team/
--
Will code for cotton.

[2/2] secur32/tests: Fix GetUserNameEx() tests (try 2)

[Paul TBBle Hampson]

On Tue, Apr 14, 2009 at 01:17:13PM +0200, Kai Blin wrote:
> On Tuesday 14 April 2009 12:39:51 Ge van Geldorp wrote:
> Hi Ge,
> > > Why did you remove:
> > > -    if (0) /* Crashes on Windows */
> > > -        rc = pGetUserNameExW(NameSamCompatible, NULL, NULL);
> > > -
> > > The point of these additions was to show/document that Windows crashes
> > > when called like this.
> > Because Alexandre told me for an earlier patch to remove tests that crash
> > on Windows:
> > http://www.winehq.org/pipermail/wine-devel/2009-January/072010.html
> What Paul meant is that as this test is behind an if(0), it's not going to be 
> run anyway. So all it does is to serve as documentation of Windows behaviour.
> Arguably, this could just be a comment, ideally in the implementation of 
> GetUserNameExW, but in general it's nice to have this sort of information.

Depending on the importance of the crash, surely an
exception-handler-type test would document that it crashes on Windows
_and_ whether it crashes or not on Wine, and catch a change in that
status by either party?

-- 
Paul "TBBle" Hampson, Paul.Hampson(a)Pobox.com

Shorter .sig for a more eco-friendly paperless office.