[linux-cifs-client] Re: cifs patch: make cifs_rename handle -EACCES errors

[Jeff Layton]

On Sun, 19 Oct 2008 20:01:04 -0500
"Steve French" <smfrench at gmail.com> wrote:

> Looking at your cifs rename patch to add handling for servers with
> bugs that return EACCES instead of EEXIST, I see multiple problems
> 
> - if after renaming the target to a temp name, setfiledisposition (of
> the delete on close flag) fails we are going to "leak" files

I'm not seeing this. As best I can tell, we're always closing the file.
Could you clarify?

> - checkpatch throws three warnings (some existing long lines but we
> might as well fix them ...)
> 

Ugh, good point. I've not been checking these with checkpatch like I
should be. I'll make a sweep of them tomorrow and clean things up.

> - in the claise
>              if (rc == -EACCES || rc == -EEXIST)
>    if return code is EEXIST the return code gets converted from EEXIST
> to EACCESS if open of the target fails (even if access is not the
> problem)

Another good point. I'll go over this tomorrow and fix it to preserve the
rc correctly.

> - It doesn't handle checking for the hardlinked file case for Windows
> (I have code for this, but it needs to be checked before the rename
> presumably).
> 

Yep. The existing code doesn't do that either though. It's probably
best to do that with a separate patch (and maybe in a separate function
that we can call from cifs_rename?)

--
Jeff Layton <jlayton at redhat.com>