Re: Write problem with MS Office 2003

[Juan Lang]

Hi Stefan,

--- Stefan D�siner  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/

Write problem with MS Office 2003

[Stefan Dösinger]

Am Dienstag, 24. Mai 2005 17:10 schrieb Juan Lang:
> Hi Stefan,
>
> --- Stefan Dösiner  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

Write problem with MS Office 2003

[Michael Jung]

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,

Write problem with MS Office 2003

[Michael Jung]

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,

Write problem with MS Office 2003

[Juan Lang]

--- Stefan D�singer  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/

Write problem with MS Office 2003

[Stefan Dösinger]

Am Dienstag, 24. Mai 2005 21:40 schrieb Juan Lang:
> --- Stefan Dösinger  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

Write problem with MS Office 2003

[Michael Jung]

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
[e-mail]

Write problem with MS Office 2003

[Stefan Dösinger]

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.

Write problem with MS Office 2003

[Michael Jung]

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,

Write problem with MS Office 2003

[Stefan Dösinger]

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

Write problem with MS Office 2003

[Michael Jung]

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,

Write problem with MS Office 2003

[Stefan Dösinger]

> 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

Write problem with MS Office 2003

[Michael Jung]

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,

Write problem with MS Office 2003

[Dimi Paun]

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 < ....

Write problem with MS Office 2003

[Stefan Dösinger]

Am Donnerstag, 26. Mai 2005 11:13 schrieb Dimi Paun:
> 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 < ....

That's it, thanks