On Mon, 2018-10-08 at 07:14 -0500, Goldwyn Rodrigues wrote:
>
> > >
> > > + if (!(file->f_mode & FMODE_READ)) {
> > > + struct file *f;
> >
> > I would define "struct file *f = file" above, at the beginning of
> > function, and "free(f)" below, without modifying "file".
>
> I suppose you mean fput(f).
yes
> Okay, if it makes code more understandable.
Thanks
>
> >
> > > + int flags = file->f_flags & ~(O_WRONLY | O_APPEND | O_TRUNC | O_CREAT | O_NOCTTY | O_EXCL);
> >
> > Doesn't O_RDONLY need to be added?
>
> No. O_RDONLY is zero. But I think I should add it for readability. The
> compiler will optimize it eventually.
>
> > Please fix the line length.
> >
> >
> > > + f = dentry_open(&file->f_path, flags, file->f_cred);
> > > + if (IS_ERR(f))
> > > + return PTR_ERR(f);
It's late in the release cycle to be making this change. Would it
make sense for now to fallback to modifying the original file
descriptor on failure and emit a message?
Mimi
> > > + read = 1;
> > > + file = f;
> >
> > With the above change, no need to modify "file".
>
