Re: [PATCH v6 4/4] fsnotify: don't merge events FS_OPEN_PERM and FS_OPEN_EXEC_PERM

[Matthew Bobrowski <mbobrowski@mbobrowski.org>]

On Wed, Nov 07, 2018 at 03:15:53PM +0100, Jan Kara wrote:
> On Wed 07-11-18 13:30:55, Amir Goldstein wrote:
> > On Wed, Nov 7, 2018 at 1:18 PM Matthew Bobrowski
> > <mbobrowski@mbobrowski.org> wrote:
> > >
> > > Permission events are not to be consolidated into a single event mask.
> > > In order for this to not happen, we require additional calls to
> > > fsnotify_parent() and fsnotify() within the fsnotify_perm() when the
> > > conditon to set FS_OPEN_EXEC_PERM is evaluated to true.
> > >
> > 
> > That shouldn't be a separate patch. it should be squashed into the patch
> > introducing FS_OPEN_EXEC_PERM there is no reason to have an
> > interim commit where events are merged.
> 
> Agreed.
> 
> > > To simplify the code that provides this functionality a simple wrapper
> > > fsnotify_path() has been defined to keep things nice and clean. Other
> > > functions that used the same fsnotify_parent()/fsnotify() call
> > > combination have been updated to use the simplified fsnotify_path()
> > > wrapper.
> > >
> > 
> > And this should be a separate re-factoring patch.
> 
> And agreed too. You can put this refactoring commit before the one
> introducing FS_OPEN_EXEC_PERM to make your life simpler...

OK, no problem.

> > return should be in newline - just was just me hand writing a patch in email...
> > 
> > After making these small fixes, you may add to patches:
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks Amir!

I will send through v7 shortly.

-- 
Matthew Bobrowski <mbobrowski@mbobrowski.org>