[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