[shellpath.c]: minor fix to Rolf's patch

[Rolf Kalbermatter]

 wrote:
> The latest patch from Rolf Kalbermatter on shellpath.c was breaking the installation of wine.inf.
>
> wine rundll32 setupapi.dll,InstallHinfSection DefaultInstall 128 d:\wine.inf
>
> was returning errors about being unable to create some dirs.  In facts these errors was concerning 
> already existing directories in SHGetFolderPathW.

I believe this patch is not fully correct. I noticed these errors too after the patch was applied,
but did a little more investigation. SHCreateDirectoryEx should never be called if the directory
already exists since this is explicitedly tested before the call to the function with PathFileExistsW().

Also inside the SHCreateDirectoryEx function I saw that the first call to CreateDirectory fails since
intermediate directories are missing and then in the recursive call to CreateDirectory every single
call returns ERROR_ALREAY_EXISTS on my system in spite of that it actually creates the last two or
three directories in there.

My investigations showed that in spite of that some of the the top level directories do not exist, the
function NtCreateFile() which is ultimately called does seem to return an according NT status when
called from SHGetFolderPath() but does create the directory nevertheless.

Strange thing is that I can't reproduce this behaviour when calling SHCreateDirectoryEx() with many
different parameters from an extended test in tests/shfileop.c

So I believe the root cause really seems somewhere else, namely NTCreateFile or maybe wineserver itself,
but I hadn't time to further go into details.
> I think it is not necessary to care about that in this function so I added a check on the
> type of error returned by SHCreateDirectoryExW.

ShCreateDirectory should fail with that error if the directory really already existed.

Rolf Kalbermatter

[Fabrice Ménard]

>  wrote:
> > The latest patch from Rolf Kalbermatter on shellpath.c was breaking the installation of wine.inf.
> >
> > wine rundll32 setupapi.dll,InstallHinfSection DefaultInstall 128 d:\wine.inf
> >
> > was returning errors about being unable to create some dirs.  In facts these errors was concerning 
> > already existing directories in SHGetFolderPathW.
>
> I believe this patch is not fully correct. I noticed these errors too after the patch was applied,
> but did a little more investigation. SHCreateDirectoryEx should never be called if the directory
> already exists since this is explicitedly tested before the call to the function with PathFileExistsW().

I came to the same conclusion as you.  In fact, the SHCreateDirectoryEx function is never called if the "entire" 
path already exists (due to the first PathFileExistsW).  The problem comes, as you saw it, with the intermediate 
components of the path.
> Also inside the SHCreateDirectoryEx function I saw that the first call to CreateDirectory fails since
> intermediate directories are missing and then in the recursive call to CreateDirectory every single
> call returns ERROR_ALREAY_EXISTS on my system in spite of that it actually creates the last two or
> three directories in there.
> My investigations showed that in spite of that some of the the top level directories do not exist, the
> function NtCreateFile() which is ultimately called does seem to return an according NT status when
> called from SHGetFolderPath() but does create the directory nevertheless.
>
> Strange thing is that I can't reproduce this behaviour when calling SHCreateDirectoryEx() with many
> different parameters from an extended test in tests/shfileop.c
>
> So I believe the root cause really seems somewhere else, namely NTCreateFile or maybe wineserver itself,
> but I hadn't time to further go into details.
> > I think it is not necessary to care about that in this function so I added a check on the
> > type of error returned by SHCreateDirectoryExW.
>
> ShCreateDirectory should fail with that error if the directory really already existed.
>
> Rolf Kalbermatter

I agree that SHCreateDirectory should fail, but I'm not convinced it is crucial int the specific
SHGetFolderPathW function. But I may be wrong.

Anyway, I'll try to see if I can find a better solution.

Regards,

[Rolf Kalbermatter]

> > ShCreateDirectory should fail with that error if the directory
> > really already existed.
>
> I agree that SHCreateDirectory should fail, but I'm not convinced
> it is crucial in the specific SHGetFolderPathW function. But I may
> be wrong.

I was wondering about that too. In my case what happened when I
registered wine.inf was that it aborted after every single directory
it created because of that error.

So I had to restart it many times and did in between different tests
such as adding printing out status informations. Every time another
path got created and then when I was finally checking at NtCreateFile
level all the directories were finally created.

So I thought I would delete a few directories again but executing wine.inf
registration again did not create the missing directories anymore!!

I was very much wondering if the registration process really should abort
completely when one of the registration parts failed for some reason. It
would seem more sensible to me to continue with the rest of the registration
anyhow.

Rolf Kalbermatter