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

[Richard Palethorpe]

Hello,

Amir Goldstein <amir73il@gmail.com> writes:

> 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.

Sorry, of course you are right.

-- 
Thank you,
Richard.