[2/4] cifs: don't leave open files dangling

If we aren't going to call cifs_new_fileinfo to put the files on
the openFileList for an inode, then they should be closed -- full stop.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/dir.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

On 05/21/2010 11:55 PM, Jeff Layton wrote:
> If we aren't going to call cifs_new_fileinfo to put the files on
> the openFileList for an inode, then they should be closed -- full stop.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/dir.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index 54de8e5..e11ee2e 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -269,6 +269,8 @@ int cifs_posix_open(char *full_path, struct inode **pinode,
>  					       oflags);
>  		if (pfile_info == NULL)
>  			rc = -ENOMEM;
> +	} else {
> +		CIFSSMBClose(xid, cifs_sb->tcon, *pnetfid);

May be this not needed here as a subsequent patch pushes
cifs_new_fileinfo to the caller..?

>  	}
>  
>  posix_open_ret:
> @@ -478,11 +480,7 @@ cifs_create_set_dentry:
>  	else
>  		cFYI(1, "Create worked, get_inode_info failed rc = %d", rc);
>  
> -	/* nfsd case - nfs srv does not set nd */
> -	if ((nd == NULL) || (!(nd->flags & LOOKUP_OPEN))) {
> -		/* mknod case - do not leave file open */
> -		CIFSSMBClose(xid, tcon, fileHandle);
> -	} else if (!(posix_create) && (newinode)) {
> +	if (!posix_create && newinode) {

Here too, posix_create is going to go..

May be lining this patch after [3/4] is a good idea..? Would make the
patchset simpler, I think.


>  		struct cifsFileInfo *pfile_info;
>  		/*
>  		 * cifs_fill_filedata() takes care of setting cifsFileInfo
> @@ -492,7 +490,10 @@ cifs_create_set_dentry:
>  					       nd->path.mnt, oflags);
>  		if (pfile_info == NULL)
>  			rc = -ENOMEM;
> +	} else {
> +		CIFSSMBClose(xid, tcon, fileHandle);
>  	}

minor nit:     ^^^ unneeded braces..

> +
>  cifs_create_out:
>  	kfree(buf);
>  	kfree(full_path);

On Mon, 24 May 2010 12:20:30 +0530
Suresh Jayaraman <sjayaraman@suse.de> wrote:

> On 05/21/2010 11:55 PM, Jeff Layton wrote:
> > If we aren't going to call cifs_new_fileinfo to put the files on
> > the openFileList for an inode, then they should be closed -- full stop.
> > 
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  fs/cifs/dir.c |   11 ++++++-----
> >  1 files changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> > index 54de8e5..e11ee2e 100644
> > --- a/fs/cifs/dir.c
> > +++ b/fs/cifs/dir.c
> > @@ -269,6 +269,8 @@ int cifs_posix_open(char *full_path, struct inode **pinode,
> >  					       oflags);
> >  		if (pfile_info == NULL)
> >  			rc = -ENOMEM;
> > +	} else {
> > +		CIFSSMBClose(xid, cifs_sb->tcon, *pnetfid);
> 
> May be this not needed here as a subsequent patch pushes
> cifs_new_fileinfo to the caller..?
> 
> >  	}
> >  
> >  posix_open_ret:
> > @@ -478,11 +480,7 @@ cifs_create_set_dentry:
> >  	else
> >  		cFYI(1, "Create worked, get_inode_info failed rc = %d", rc);
> >  
> > -	/* nfsd case - nfs srv does not set nd */
> > -	if ((nd == NULL) || (!(nd->flags & LOOKUP_OPEN))) {
> > -		/* mknod case - do not leave file open */
> > -		CIFSSMBClose(xid, tcon, fileHandle);
> > -	} else if (!(posix_create) && (newinode)) {
> > +	if (!posix_create && newinode) {
> 
> Here too, posix_create is going to go..
> 
> May be lining this patch after [3/4] is a good idea..? Would make the
> patchset simpler, I think.
> 
> 
> >  		struct cifsFileInfo *pfile_info;
> >  		/*
> >  		 * cifs_fill_filedata() takes care of setting cifsFileInfo
> > @@ -492,7 +490,10 @@ cifs_create_set_dentry:
> >  					       nd->path.mnt, oflags);
> >  		if (pfile_info == NULL)
> >  			rc = -ENOMEM;
> > +	} else {
> > +		CIFSSMBClose(xid, tcon, fileHandle);
> >  	}
> 
> minor nit:     ^^^ unneeded braces..
> 
> > +
> >  cifs_create_out:
> >  	kfree(buf);
> >  	kfree(full_path);
> 
> 

Good point. I'll move this one to near the end of the set.