On 10/08/2010 11:01 PM, Jeff Layton wrote: > Eliminate the poor, misunderstood "oflags" option from cifs_new_fileinfo. > The callers mostly pass in the filp->f_flags here, except for one place > that passes in the flags after they've been converted to their SMB_O_* > equivalents. > > None of these are correct however since we're checking that value for > the presence of FMODE_READ. Luckily that only affects how the f_list is > ordered. What it really wants here is the file->f_mode, but this check > really makes no sense whatsoever. FMODE_READ will be set for O_RDWR or > O_RDONLY opens. So this in effect just moves those to the front of the > list and leaves O_WRONLY at the end. > > That might make some sense if anything actually paid attention to this > list order, but nothing does. find_readable_file and find_writable_file > both walk through the list in the same direction. > > Let's just keep this simple and just put things on the list with > list_add. > > Signed-off-by: Jeff Layton <[email protected]> > --- > fs/cifs/cifsproto.h | 3 +-- > fs/cifs/dir.c | 15 ++++----------- > fs/cifs/file.c | 5 ++--- > 3 files changed, 7 insertions(+), 16 deletions(-) > > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h > index 7f416ab..bed004c 100644 > --- a/fs/cifs/cifsproto.h > +++ b/fs/cifs/cifsproto.h > @@ -107,8 +107,7 @@ extern struct timespec cnvrtDosUnixTm(__le16 le_date, > __le16 le_time, > > extern struct cifsFileInfo *cifs_new_fileinfo(struct inode *newinode, > __u16 fileHandle, struct file *file, > - struct tcon_link *tlink, > - unsigned int oflags, __u32 oplock); > + struct tcon_link *tlink, __u32 oplock); > extern int cifs_posix_open(char *full_path, struct inode **pinode, > struct super_block *sb, > int mode, int oflags, > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c > index c205ec9..452c9b5 100644 > --- a/fs/cifs/dir.c > +++ b/fs/cifs/dir.c > @@ -132,7 +132,7 @@ cifs_bp_rename_retry: > > struct cifsFileInfo * > cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file > *file, > - struct tcon_link *tlink, unsigned int oflags, __u32 oplock) > + struct tcon_link *tlink, __u32 oplock) > { > struct dentry *dentry = file->f_path.dentry; > struct cifsFileInfo *pCifsFile; > @@ -160,13 +160,7 @@ cifs_new_fileinfo(struct inode *newinode, __u16 > fileHandle, struct file *file, > list_add(&pCifsFile->tlist, &(tlink_tcon(tlink)->openFileList)); > pCifsInode = CIFS_I(newinode); > if (pCifsInode) { > - /* if readable file instance put first in list*/ > - if (oflags & FMODE_READ) > - list_add(&pCifsFile->flist, &pCifsInode->openFileList); > - else > - list_add_tail(&pCifsFile->flist, > - &pCifsInode->openFileList); > - > + list_add(&pCifsFile->flist, &pCifsInode->openFileList);
find_readable_file() assumes that if it is a write-only file, it will be in the tail and it can break out of the loop. That needs to be fixed as well? -- Suresh Jayaraman -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
