Re: dlls/shell32/pidl.c -- adjust to unsignedness of a type

[Gerald Pfeifer]

On Sat, 16 Feb 2008, Alexandre Julliard wrote:
 The test should probably be removed completely since there's already a
 default for it in the switch() that follows.

I was thinking of this, but wanted to go for the smaller patch initially,
but your wish is my command ;-). 

Updated patch below.  Nearly everything here is whitespace changes; the 
second hunk only removes if (type = 0  type = 2) { and }.

Gerald

ChangeLog:
Adjust a format specifier and remove a redundant range check in
ILGetDisplayNameExW().

Index: pidl.c
===
RCS file: /home/wine/wine/dlls/shell32/pidl.c,v
retrieving revision 1.159
diff -u -3 -p -r1.159 pidl.c
--- pidl.c  16 Feb 2008 15:58:53 -  1.159
+++ pidl.c  17 Feb 2008 18:44:54 -
@@ -97,7 +97,7 @@ BOOL WINAPI ILGetDisplayNameExW(LPSHELLF
 STRRET strret;
 DWORD flag;
 
-TRACE(%p %p %p %d\n, psf, pidl, path, type);
+TRACE(%p %p %p %x\n, psf, pidl, path, type);
 
 if (!pidl || !path)
 return FALSE;
@@ -109,46 +109,44 @@ BOOL WINAPI ILGetDisplayNameExW(LPSHELLF
 return FALSE;
 }
 
-if (type = 0  type = 2)
+switch (type)
 {
-switch (type)
+case ILGDN_FORPARSING:
+flag = SHGDN_FORPARSING | SHGDN_FORADDRESSBAR;
+break;
+case ILGDN_NORMAL:
+flag = SHGDN_NORMAL;
+break;
+case ILGDN_INFOLDER:
+flag = SHGDN_INFOLDER;
+break;
+default:
+FIXME(Unknown type parameter = %x\n, type);
+flag = SHGDN_FORPARSING | SHGDN_FORADDRESSBAR;
+break;
+}
+
+if (!*(const WORD*)pidl || type == ILGDN_FORPARSING)
+{
+ret = IShellFolder_GetDisplayNameOf(lsf, pidl, flag, strret);
+if (SUCCEEDED(ret))
 {
-case ILGDN_FORPARSING:
-flag = SHGDN_FORPARSING | SHGDN_FORADDRESSBAR;
-break;
-case ILGDN_NORMAL:
-flag = SHGDN_NORMAL;
-break;
-case ILGDN_INFOLDER:
-flag = SHGDN_INFOLDER;
-break;
-default:
-FIXME(Unknown type parameter = %x\n, type);
-flag = SHGDN_FORPARSING | SHGDN_FORADDRESSBAR;
-break;
+if(!StrRetToStrNW(path, MAX_PATH, strret, pidl))
+ret = E_FAIL;
 }
-if (!*(const WORD*)pidl || type == ILGDN_FORPARSING)
+}
+else
+{
+ret = SHBindToParent(pidl, IID_IShellFolder, (LPVOID*)psfParent, 
pidllast);
+if (SUCCEEDED(ret))
 {
-ret = IShellFolder_GetDisplayNameOf(lsf, pidl, flag, strret);
+ret = IShellFolder_GetDisplayNameOf(psfParent, pidllast, flag, 
strret);
 if (SUCCEEDED(ret))
 {
-if(!StrRetToStrNW(path, MAX_PATH, strret, pidl))
+if(!StrRetToStrNW(path, MAX_PATH, strret, pidllast))
 ret = E_FAIL;
 }
-}
-else
-{
-ret = SHBindToParent(pidl, IID_IShellFolder, (LPVOID*)psfParent, 
pidllast);
-if (SUCCEEDED(ret))
-{
-ret = IShellFolder_GetDisplayNameOf(psfParent, pidllast, flag, 
strret);
-if (SUCCEEDED(ret))
-{
-if(!StrRetToStrNW(path, MAX_PATH, strret, pidllast))
-ret = E_FAIL;
-}
-IShellFolder_Release(psfParent);
-}
+IShellFolder_Release(psfParent);
 }
 }
 

Re: dlls/shell32/pidl.c -- adjust to unsignedness of a type

[Alexandre Julliard]

Gerald Pfeifer [EMAIL PROTECTED] writes:

 @@ -109,7 +109,7 @@ BOOL WINAPI ILGetDisplayNameExW(LPSHELLF
  return FALSE;
  }
  
 -if (type = 0  type = 2)
 +if (type = 2)
  {
  switch (type)
  {

The test should probably be removed completely since there's already a
default for it in the switch() that follows.

-- 
Alexandre Julliard
[EMAIL PROTECTED]

Re: dlls/shell32/pidl.c

[Michael Jung]

Hi Ge,

On Saturday 13 August 2005 20:55, Ge van Geldorp wrote:
  dwAttributes = SFGAO_FILESYSTEM;
  hr = IShellFolder_GetAttributesOf(psfFolder, 1, pidlLast,
 dwAttributes); -if (FAILED(hr) || !(dwAttributes  SFGAO_FILESYSTEM))
 return FALSE; +if (FAILED(hr) || !(dwAttributes  SFGAO_FILESYSTEM)) {
 +IShellFolder_Release(psfFolder);
 +ILFree((LPITEMIDLIST) pidlLast);
 +return FALSE;
 +}


I had another look at this after realising that Alexandre did'nt apply the 
patch. SHBindToParent does not allocate a new PIDL for pidlLast, but returns 
a pointer to right location in pidl. This means you should not free it. 
There's still the problem that the shell folder isn't released in failure 
cases.

Sorry, I didn't realize this. Once again I'm impressed that Alexandre always 
catches those things.

Bye, 
-- 
Michael Jung
[EMAIL PROTECTED]

RE: dlls/shell32/pidl.c

[Ge van Geldorp]

 From: Michael Jung
 
 SHBindToParent does not allocate a 
 new PIDL for pidlLast, but returns a pointer to right 
 location in pidl. This means you should not free it. 
 There's still the problem that the shell folder isn't 
 released in failure cases.
 
 Sorry, I didn't realize this.

I feel a bit silly for not catching it either. MSDN is very clear about
this. I've resubmitted a patch which only releases the interface pointer.

Gé van Geldorp.