Hi Stefan,
--- Stefan D�siner [email protected] wrote:
How do I handle apidl == NULL? As far as I understand, apidl specifies a list of folders/files to be checked, right? If apidl == 0, what folder should I check. Is there some 'current folder' in the IShellFolder class? I didn't find any.
Right, apidl can't be NULL if cidl is nonzero. If cidl is zero, you probably need the check to be in the caller. E.g. in shfldr_fs.c, the current folder is what calls SHELL32_GetItemAttributes, from IShellFolder_fnGetAttributesOf. Perhaps the dwAttributes member that's passed to InitializeEx sets the correct attributes for the current folder? I'm not sure.
--Juan
__________________________________ Do you Yahoo!? Yahoo! Small Business - Try our new Resources site http://smallbusiness.yahoo.com/resources/
Am Dienstag, 24. Mai 2005 17:10 schrieb Juan Lang:
Hi Stefan,
--- Stefan Dösiner [email protected] wrote:
How do I handle apidl == NULL? As far as I understand, apidl specifies a list of folders/files to be checked, right? If apidl == 0, what folder should I check. Is there some 'current folder' in the IShellFolder class? I didn't find any.
Right, apidl can't be NULL if cidl is nonzero. If cidl is zero, you probably need the check to be in the caller. E.g. in shfldr_fs.c, the current folder is what calls SHELL32_GetItemAttributes, from IShellFolder_fnGetAttributesOf. Perhaps the dwAttributes member that's passed to InitializeEx sets the correct attributes for the current folder? I'm not sure.
This->pidlRoot looks promissing
Checking against This->pidlRoot makes writing work. On an write-protected folder, this check still succeeds(the read-only flag is removed). Perhaps a test with windows is needed. If I try to write to an write-protected folder in win2k, Windows simply ingores the write protection. Under Wine there's an error later on if the file can't be written to.
Is the attached patch correct? I am not sure because I don't know much about these things, so I am not submitting it to wine-patches yet. Any suggestions?
Stefan
On Tuesday 24 May 2005 22:45, Stefan Dösinger wrote:
On an write-protected folder, this check still succeeds(the read-only flag is removed).
This is from ntdll/directory, line 792:
if (!(st.st_mode & (S_IWUSR | S_IWGRP | S_IWOTH))) info->FileAttributes |= FILE_ATTRIBUTE_READONLY;
So the readonly flag is set only if nobody (not even the file owner or members of the file owner's group) have read privileges.
So it's not a problem in the shell folders, it's a problem in ntdll. I wonder if this is correctly implemented. Should'nt we check if the current user has read privileges? Or is this the behaviour that windows shows on an ntfs filesystem?
Bye,
On Tuesday 24 May 2005 22:45, Stefan Dösinger wrote:
Is the attached patch correct? I am not sure because I don't know much about these things, so I am not submitting it to wine-patches yet. Any suggestions?
Just realized that there's still a problem with your patch. At the moment, you are querying the attributes of the drive.
You probably have to bind to the folder's parent with SHBindToParent. Then call SHELL32_GetItemAttributes on the parent, with the last element of the original pidl (which SHBindToParent provides for you).
Bye,
--- Stefan D�singer [email protected] wrote:
Checking against This->pidlRoot makes writing work.
<snip>
Is the attached patch correct?
That appears as if it would work. Although I must say my curiosity isn't quite satisfied. There's the comment in shlfldr_fs.c about dwAttributes, saying it should be used. And yet it's never initialized.
So it seems to me you could remove dwAttributes and the bad comment next to it, or you could: - initialize it in Initialize and InitializeEx, using SHELL32_GetItemAttributes with This->pidlRoot - use This->dwAttributes in fnGetAttributesOf when cidl == 0 The latter way would be more performant perhaps.
Also, whichever method you choose, please make sure to make the same change in the remaining implementations of IShellFolder.
Thanks, --Juan
__________________________________ Do you Yahoo!? Yahoo! Small Business - Try our new Resources site http://smallbusiness.yahoo.com/resources/
Am Dienstag, 24. Mai 2005 21:40 schrieb Juan Lang:
--- Stefan Dösinger [email protected] wrote:
Checking against This->pidlRoot makes writing work.
<snip>
Is the attached patch correct?
That appears as if it would work. Although I must say my curiosity isn't quite satisfied. There's the comment in shlfldr_fs.c about dwAttributes, saying it should be used. And yet it's never initialized.
So it seems to me you could remove dwAttributes and the bad comment next to it, or you could:
- initialize it in Initialize and InitializeEx, using
SHELL32_GetItemAttributes with This->pidlRoot
- use This->dwAttributes in fnGetAttributesOf when cidl == 0
The latter way would be more performant perhaps.
That sounds right for me. Any suggestions to the following patch? Now the same thing for the other shfldr_s, right? shfldr_desktop.c, shfldr_mycomp.c and shfldr_unixfs.c
Stefan
Hi Stefan,
On Wednesday 25 May 2005 18:13, Stefan Dösinger wrote:
Any suggestions to the following patch?
+ SHELL32_GetItemAttributes (_IShellFolder_ (This), This->pidlRoot, &dwAttributes);
This->pidlRoot is the ITEMIDLIST of all SHITEMIDs starting from the root of the shell namespace up to the current folder. If you take a look into SHELL32_GetItemAttributes, it calls _ILIsDrive, _ILGetGUIDPointer and the like on the pidl. Those function inspect the first SHITEMID in the ITEMIDLIST only (see pidl.c). This means they will inspect the SHITEMID which represents the drive, which the ITEMIDLIST ist based on.
What you really need to do is to call SHBindToParent. This will give you a pointer to the current folders parent folder as well as the last SHITEMID of the ITEMIDLIST. Call SHELL32_GetItemAttributes with the parent folder and this last SHITEMID.
As to the caching of the file attributes: Those are already cached in the SHITEMID (see _ILGetFileAttributes in pidl.c). So you are adding a redundant level of caching and thus unnecessary complexity. IMHO it would be better to remove the IGenericFSImpl's dwAttributes member and just call SHELL32_GetItemAttributes.
Bye, -- Michael Jung [email protected]
Hi,
Hi Stefan,
On Wednesday 25 May 2005 18:13, Stefan Dösinger wrote:
Any suggestions to the following patch?
- SHELL32_GetItemAttributes (_IShellFolder_ (This), This->pidlRoot,
&dwAttributes);
This->pidlRoot is the ITEMIDLIST of all SHITEMIDs starting from the root of the shell namespace up to the current folder. If you take a look into SHELL32_GetItemAttributes, it calls _ILIsDrive, _ILGetGUIDPointer and the like on the pidl. Those function inspect the first SHITEMID in the ITEMIDLIST only (see pidl.c). This means they will inspect the SHITEMID which represents the drive, which the ITEMIDLIST ist based on.
Just a question if I understood that correctly So PidlRoot represents a list of Folders, from the root to the current folder. For C:\somedir\dirx this would more or less mean C:\ somedir\ dirx\ (or even starting from Desktop. Msdn somewhere mentiones the Shell namespace)
What you really need to do is to call SHBindToParent. This will give you a pointer to the current folders parent folder as well as the last SHITEMID of the ITEMIDLIST. Call SHELL32_GetItemAttributes with the parent folder and this last SHITEMID.
Can I use pidlRoot(passed by the function caller) for SHBindToParent? Now I have SHBindToParent(This->pidlRoot, &IID_IShellFolder, (LPVOID*)&psfParent, (LPCITEMIDLIST*)&rpidl);
later I call SHELL32_GetItemAttributes with SHELL32_GetItemAttributes (psfParent, rpidl, (DWORD *) &This->dwAttributes);
This causes I crash. If I understand correctly, I have to set the flags I want to test in This->dwAttributes before I call SHELL32_GetItemAttributes. If I set it to 0xffffffff(I want to test for all flags) of all flags listed as supported flags in shfolder.c, I get 0xf0000144(the flags a drive supports, oh, surprise) every time and a stack overflow later on.
As to the caching of the file attributes: Those are already cached in the SHITEMID (see _ILGetFileAttributes in pidl.c). So you are adding a redundant level of caching and thus unnecessary complexity. IMHO it would be better to remove the IGenericFSImpl's dwAttributes member and just call SHELL32_GetItemAttributes.
You mean that I can pass &This->dwAttributes directly to SHELL32_GetItemAttributes?
Thanks for your help, Stefan
PS: A summary of the relevan lines: (The modified function is IFSFldr_PersistFolder3_InitializeEx)
hr = SHBindToParent(This->pidlRoot, &IID_IShellFolder, (LPVOID*)&psfParent, (LPCITEMIDLIST*)&rpidl); if(SUCCEEDED(hr)) { This->dwAttributes = 0xffffffff;
SHELL32_GetItemAttributes (psfParent, rpidl, (DWORD *) &This->dwAttributes); ERR("Attribs: 0x%08lx\n", (DWORD) This->dwAttributes); } else This->dwAttributes = 0;
This->PidlRoot is set by existing code to ILClone (pidlRoot), and pidlRoot is a parameter to the function.
On Wednesday 25 May 2005 20:38, Stefan Dösinger wrote:
Just a question if I understood that correctly So PidlRoot represents a list of Folders, from the root to the current folder. For C:\somedir\dirx this would more or less mean C:\ somedir\ dirx\
Yes, that's correct.
Can I use pidlRoot(passed by the function caller) for SHBindToParent? Now I have SHBindToParent(This->pidlRoot, &IID_IShellFolder, (LPVOID*)&psfParent, (LPCITEMIDLIST*)&rpidl);
Looks good to me.
later I call SHELL32_GetItemAttributes with SHELL32_GetItemAttributes (psfParent, rpidl, (DWORD *) &This->dwAttributes);
This should work, but you should do this in GetAttributesOf and not in Intialize[Ex], passing the rfgInOut parameter instead of &This->dwAttributes. See below.
This causes I crash.
Ooops, no idea. Sorry. Could you send me the complete diff?
If I understand correctly, I have to set the flags I want to test in This->dwAttributes before I call SHELL32_GetItemAttributes. If I set it to 0xffffffff(I want to test for all flags) of all flags listed as supported flags in shfolder.c, I get 0xf0000144(the flags a drive supports, oh, surprise) every time and a stack overflow later on.
If you use rfgInOut you don't have to set dwAttributes to 0xffffffff. The caller specifies, which flags it is interested in.
As to the caching of the file attributes: Those are already cached in the SHITEMID (see _ILGetFileAttributes in pidl.c). So you are adding a redundant level of caching and thus unnecessary complexity. IMHO it would be better to remove the IGenericFSImpl's dwAttributes member and just call SHELL32_GetItemAttributes.
You mean that I can pass &This->dwAttributes directly to SHELL32_GetItemAttributes?
I meant that you should remove the dwAttributes member from IGenericFSFolder and call SHELL32_GetItemAttributes in shfldr_fs's GetAttributesOf method passing the rfgInOut parameter. You don't have to modify the Initialize[Ex] methods.
That said, this is the preferable option in my opinion. At the end of the day, it's up to you (or in effect Alexandre).
Thanks for your help, Stefan
PS: A summary of the relevan lines: (The modified function is IFSFldr_PersistFolder3_InitializeEx)
hr = SHBindToParent(This->pidlRoot, &IID_IShellFolder,(LPVOID*)&psfParent, (LPCITEMIDLIST*)&rpidl); if(SUCCEEDED(hr)) { This->dwAttributes = 0xffffffff;
SHELL32_GetItemAttributes (psfParent, rpidl, (DWORD *)&This->dwAttributes); ERR("Attribs: 0x%08lx\n", (DWORD) This->dwAttributes); } else This->dwAttributes = 0;
This->PidlRoot is set by existing code to ILClone (pidlRoot), and pidlRoot is a parameter to the function.
I don't see why this should crash (also I think that it belongs in GetAttributesOf instead of InitializeEx). Oh, perhaps I know the reason for the crash: Do you do the same stuff in Initialize (the non 'Ex' versions). If not and the shellfolder is initialized via 'Initialize' instead of 'InitializeEx' the value of the dwAttributes is basically random, which might cause a crash. Nevertheless, if you move it to GetAttributes of, those are of course non problems.
Bye,
Hi,
I meant that you should remove the dwAttributes member from IGenericFSFolder and call SHELL32_GetItemAttributes in shfldr_fs's GetAttributesOf method passing the rfgInOut parameter. You don't have to modify the Initialize[Ex] methods.
That said, this is the preferable option in my opinion. At the end of the day, it's up to you (or in effect Alexandre).
Done so, see the attached patch. No crash, Word is happy. If the folder I want to write to _is_ write protected, the new code also doesn't return the write protection flag, but it looks like Win2k does the same for write protected folders on VFAT file systems(haven't tested ntfs)
Any further suggestions? If not, I'll modify the other shflrds and send a patch to wine-patches ;-)
Greetings, Stefan
On Thursday 26 May 2005 13:18, Stefan Dösinger wrote:
Any further suggestions? If not, I'll modify the other shflrds and send a patch to wine-patches ;-)
Looks good to me.
Two style things:
First, I would move the psfParent and rpidl variable declarations into the then-branch of the if statement, since this is the only place they are used. But this is of course personal preference.
Second, please use the -pu flags when generating the diff. This makes the diffs more robust and more easily readable for humans.
Bye,
First, I would move the psfParent and rpidl variable declarations into the then-branch of the if statement, since this is the only place they are used. But this is of course personal preference.
Good, done so. I always thought this was a C++ invention.
Second, please use the -pu flags when generating the diff. This makes the diffs more robust and more easily readable for humans.
One more question: I create my diffs with cvs diff -pu, but if I want to apply them with patch, I have to specify the filename:
patch -p 1 -R < /home/stefan/Desktop/shell32.diff (Just to test if the patch is OK) can't find file to patch at input line 8 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: dlls/shell32/shfldr_desktop.c |=================================================================== |RCS file: /home/wine/wine/dlls/shell32/shfldr_desktop.c,v |retrieving revision 1.39 |diff -p -u -r1.39 shfldr_desktop.c |--- dlls/shell32/shfldr_desktop.c 23 May 2005 16:31:42 -0000 1.39 |+++ dlls/shell32/shfldr_desktop.c 26 May 2005 10:28:22 -0000 -------------------------- File to patch:
I created the diffs from the wine root, and I apply the patches in the sources root directory. If I specify the filedlls/shell32/shfldr_desktop.c, the patch applies fine.
The new patch is attached for reference.
Any hints, Stefan
On Thursday 26 May 2005 14:40, Stefan Dösinger wrote:
I created the diffs from the wine root, and I apply the patches in the sources root directory. If I specify the filedlls/shell32/shfldr_desktop.c, the patch applies fine.
You should create as well as apply the patches in the top level wine directory. When you apply a patch use "patch -p0 < foobar.diff". I don't see the reason to do this, but if you would like to apply the patch in dlls/shell32, you have to call "patch -p2 < foobar.diff". This will remove the first two sub-directories from every file name in the diff.
The new patch is attached for reference.
Applies fine for me in the top level wine directory with "patch -p0 < shell32.diff"
Bye,
On Thu, 2005-05-26 at 12:40 +0000, Stefan Dösinger wrote:
patch -p 1 -R < /home/stefan/Desktop/shell32.diff (Just to test if
^^^^ patch -p0 -R < ....
-
Dimi Paun -
Juan Lang -
Michael Jung -
Stefan Dösinger