Re: Create our own temp directory to make sure it's not the Windows directory.

[Paul Vriens]

Hi Alexandre,

This one introduced a test failure on several W2K, XP and W2K3 boxes. No 
  idea why (yet) though.

Create our own temp directory to make sure it's not the Windows directory.

[Nicolas Le Cam]

2009/7/24 Paul Vriens :
> Hi Alexandre,
>
> This one introduced a test failure on several W2K, XP and W2K3 boxes. No
>  idea why (yet) though.
>
> --
> Cheers,
>
> Paul.

Hi Paul, Alexandre,

It seems to only be a short vs long pathname problem.

Create our own temp directory to make sure it's not the Windows directory.

[Paul Vriens]

Nicolas Le Cam wrote:
> 2009/7/24 Paul Vriens :
> > Hi Alexandre,
> >
> > This one introduced a test failure on several W2K, XP and W2K3 boxes. No
> >  idea why (yet) though.
> >
> > --
> > Cheers,
> >
> > Paul.
>
> Hi Paul, Alexandre,
>
> It seems to only be a short vs long pathname problem.

But where? There doesn't seem to be much difference in the tmpdir 
variable before or after this patch (both short pathname, with the one 
after the patch being slightly longer of course).

Create our own temp directory to make sure it's not the Windows directory.

[Nicolas Le Cam]

2009/7/24 Paul Vriens :
> Nicolas Le Cam wrote:
> > 2009/7/24 Paul Vriens :
> > > Hi Alexandre,
> > >
> > > This one introduced a test failure on several W2K, XP and W2K3 boxes. No
> > >  idea why (yet) though.
> > >
> > > --
> > > Cheers,
> > >
> > > Paul.
> >
> > Hi Paul, Alexandre,
> >
> > It seems to only be a short vs long pathname problem.
>
> But where? There doesn't seem to be much difference in the tmpdir variable
> before or after this patch (both short pathname, with the one after the
> patch being slightly longer of course).
>
> --
> Cheers,
>
> Paul.

I will have a look at it this evening as one of the failing boxes is mine.
Perhaps GetTempFileName is now returning a short path to not override
MAX_PATH characters.

Create our own temp directory to make sure it's not the Windows directory.

[Paul Vriens]

Nicolas Le Cam wrote:
> 2009/7/24 Paul Vriens :
> > Nicolas Le Cam wrote:
> > > 2009/7/24 Paul Vriens :
> > > > Hi Alexandre,
> > > >
> > > > This one introduced a test failure on several W2K, XP and W2K3 boxes. No
> > > >  idea why (yet) though.
> > > >
> > > > --
> > > > Cheers,
> > > >
> > > > Paul.
> > >
> > > Hi Paul, Alexandre,
> > >
> > > It seems to only be a short vs long pathname problem.
> >
> > But where? There doesn't seem to be much difference in the tmpdir variable
> > before or after this patch (both short pathname, with the one after the
> > patch being slightly longer of course).
> >
> > --
> > Cheers,
> >
> > Paul.
>
> I will have a look at it this evening as one of the failing boxes is mine.
> Perhaps GetTempFileName is now returning a short path to not override
> MAX_PATH characters.

My W2K fails as well, so I'm able to test something.

From test.winehq.org one can see that this test only seems to fail for 
boxes with a TMP/TEMP variable that's converted to a short notation (see 
the kernel32/path tests for that).

Adding a "GetLongPathName(filename, filename, MAX_PATH);" hack just 
before the failing test fixes the failure.

So, for some reason this test now fails whereas before it would succeed 
and the differences between before and after the patch are not that big.

Create our own temp directory to make sure it's not the Windows directory.

[Paul Vriens]

Paul Vriens wrote:
> Nicolas Le Cam wrote:
> > 2009/7/24 Paul Vriens :
> > > Nicolas Le Cam wrote:
> > > > 2009/7/24 Paul Vriens :
> > > > > Hi Alexandre,
> > > > >
> > > > > This one introduced a test failure on several W2K, XP and W2K3 
> > > > > boxes. No
> > > > >  idea why (yet) though.
> > > > >
> > > > > -- 
> > > > > Cheers,
> > > > >
> > > > > Paul.
> > > >
> > > > Hi Paul, Alexandre,
> > > >
> > > > It seems to only be a short vs long pathname problem.
> > >
> > > But where? There doesn't seem to be much difference in the tmpdir 
> > > variable
> > > before or after this patch (both short pathname, with the one after the
> > > patch being slightly longer of course).
> > >
> > > -- 
> > > Cheers,
> > >
> > > Paul.
> >
> > I will have a look at it this evening as one of the failing boxes is 
> > mine.
> > Perhaps GetTempFileName is now returning a short path to not override
> > MAX_PATH characters.
>
> My W2K fails as well, so I'm able to test something.
>
> From test.winehq.org one can see that this test only seems to fail for 
> boxes with a TMP/TEMP variable that's converted to a short notation (see 
> the kernel32/path tests for that).
>
> Adding a "GetLongPathName(filename, filename, MAX_PATH);" hack just 
> before the failing test fixes the failure.
>
> So, for some reason this test now fails whereas before it would succeed 
> and the differences between before and after the patch are not that big.

Looks like there is a behavior changes somewhere when double backslashes 
are involved.

This fixes the issue on my W2K box:

diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c
index 5f0776a..0b06154 100644
--- a/dlls/shell32/tests/shlexec.c
+++ b/dlls/shell32/tests/shlexec.c
@@ -1527,6 +1527,7 @@ static void init_test(void)
      DeleteFileA( tmpdir );
      rc = CreateDirectoryA( tmpdir, NULL );
      ok( rc, "failed to create %s err %u\n", tmpdir, GetLastError() );
+    lstrcatA(tmpdir, "\");
      rc = GetTempFileNameA(tmpdir, "wt", 0, child_file);
      assert(rc != 0);
      init_event(child_file);

This is not a fix as it will introduce the issue with the double 
backslashes on Win95 again.

Create our own temp directory to make sure it's not the Windows directory.

[Nicolas Le Cam]

2009/7/24 Paul Vriens :
> Paul Vriens wrote:
> > Nicolas Le Cam wrote:
> > > 2009/7/24 Paul Vriens :
> > > > Nicolas Le Cam wrote:
> > > > > 2009/7/24 Paul Vriens :
> > > > > > Hi Alexandre,
> > > > > >
> > > > > > This one introduced a test failure on several W2K, XP and W2K3 boxes.
> > > > > > No
> > > > > >  idea why (yet) though.
> > > > > >
> > > > > > --
> > > > > > Cheers,
> > > > > >
> > > > > > Paul.
> > > > >
> > > > > Hi Paul, Alexandre,
> > > > >
> > > > > It seems to only be a short vs long pathname problem.
> > > >
> > > > But where? There doesn't seem to be much difference in the tmpdir
> > > > variable
> > > > before or after this patch (both short pathname, with the one after the
> > > > patch being slightly longer of course).
> > > >
> > > > --
> > > > Cheers,
> > > >
> > > > Paul.
> > >
> > > I will have a look at it this evening as one of the failing boxes is
> > > mine.
> > > Perhaps GetTempFileName is now returning a short path to not override
> > > MAX_PATH characters.
> >
> > My W2K fails as well, so I'm able to test something.
> >
> > From test.winehq.org one can see that this test only seems to fail for
> > boxes with a TMP/TEMP variable that's converted to a short notation (see the
> > kernel32/path tests for that).
> >
> > Adding a "GetLongPathName(filename, filename, MAX_PATH);" hack just before
> > the failing test fixes the failure.
> >
> > So, for some reason this test now fails whereas before it would succeed
> > and the differences between before and after the patch are not that big.
>
> Looks like there is a behavior changes somewhere when double backslashes are
> involved.
>
> This fixes the issue on my W2K box:
>
> diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c
> index 5f0776a..0b06154 100644
> --- a/dlls/shell32/tests/shlexec.c
> +++ b/dlls/shell32/tests/shlexec.c
> @@ -1527,6 +1527,7 @@ static void init_test(void)
>     DeleteFileA( tmpdir );
>     rc = CreateDirectoryA( tmpdir, NULL );
>     ok( rc, "failed to create %s err %u\n", tmpdir, GetLastError() );
> - lstrcatA(tmpdir, "\");
>
> rc = GetTempFileNameA(tmpdir, "wt", 0, child_file);
>     assert(rc != 0);
>     init_event(child_file);
>
> This is not a fix as it will introduce the issue with the double backslashes
> on Win95 again.
>
> --
> Cheers,
>
> Paul.

You were right, IShellLinkA_SetPath's behavior changes when double
backslashes are involved.

Traces before Alexandre's patch :
shlexec.c:996: filename: 'C:\DOCUME~1\nlecam\LOCALS~1\Temp\test file.shlexec'
shlexec.c:998: argvA4: 'C:\DOCUME~1\nlecam\LOCALS~1\Temp\test file.shlexec'
and after :
shlexec.c:996: filename:
'C:\DOCUME~1\nlecam\LOCALS~1\Temp\wt2.tmp\test file.shlexec'
shlexec.c:998: argvA4: 'C:\Documents and Settings\nlecam\Local
Settings\Temp\wt2.tmp\test file.shlexec'

Before this patch the created link was having a double backslashes in
its path and IShellLinkA_SetPath was only removing one of them (and
StrCmpPath was handling the backslashes differences). Now that the
path is completely valid, IShellLinkA_SetPath seems to save it as a
long pathname.

I will add tests for IShellLinkA_SetPath with short pathes and double
backslashes and fix this one with a call to GetLongPathName on tmpdir.