[LTP] [PATCH] syscalls/fanotify: misc cleanups

Amir Goldstein amir73il@gmail.com
Mon Sep 10 20:20:31 CEST 2018


On Mon, Sep 10, 2018 at 5:35 PM Richard Palethorpe <rpalethorpe@suse.de> wrote:
>
> Hello Amir,
>
> Amir Goldstein <amir73il@gmail.com> writes:
>
> > * Cleanup backup file descriptor in fanotify03
>
> Please send the fanotify03 changes on their own. It is generally best to
> send the smallest "reasonable" patch set possible to make the feeback and
> correction cycle as fast as possible.
>

ok.

> >
> > * Fix whitespace and indentation
>
> Although it is nice to clean up the whitespace, it doesn't justify
> overwritting the git history (because it makes git-blame less usable)
> and we have to verify that there are no logic changes hidden in the
> whitespace changes. It is better to mix whitespace corrections in with
> actual logic changes where we have to read every line anyway.
>

It's fine by me to drop the whitespace cleanup, although about the
argument of verifying no logic changes, git diff -w is quite useful.

> Also use "checkpatch.pl --no-tree -f" from the kernel when checking
> whitespace and style.
>
> > @@ -262,6 +262,8 @@ static void cleanup(void)
> >  {
> >       if (fd_notify > 0)
> >               SAFE_CLOSE(fd_notify);
> > +     if (fd_notify_backup > 0)
> > +             SAFE_CLOSE(fd_notify_backup);
> >  }
> >
>
> Unless I am mistaken the call to SAFE_DUP may fail leaving fd_notify
> with a valid fd and nothing in fd_notify_backup. So it is probably best
> to check and close both of them.
>

I'm not following.
Isn't "checking and closing both of them" what my fix does??

Thanks,
Amir.


More information about the ltp mailing list