From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:51366 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727691AbeJBX17 (ORCPT ); Tue, 2 Oct 2018 19:27:59 -0400 Date: Tue, 2 Oct 2018 18:43:35 +0200 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , linux-fsdevel , linux-api@vger.kernel.org Subject: Re: [PATCH][RFC] fanotify: deprecate uapi FAN_ALL_* constants Message-ID: <20181002164335.GJ9127@quack2.suse.cz> References: <20180927220112.25123-1-amir73il@gmail.com> <20181002155634.GI9127@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue 02-10-18 19:23:53, Amir Goldstein wrote: > On Tue, Oct 2, 2018 at 6:56 PM Jan Kara wrote: > > Regarding to this patch I have just two nits: > > > > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c > > > index 94b52157bf8d..e5a3c69848e4 100644 > > > --- a/fs/notify/fanotify/fanotify.c > > > +++ b/fs/notify/fanotify/fanotify.c > > > @@ -131,8 +131,8 @@ static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info, > > > !(marks_mask & FS_ISDIR & ~marks_ignored_mask)) > > > return false; > > > > > > - if (event_mask & FAN_ALL_OUTGOING_EVENTS & marks_mask & > > > - ~marks_ignored_mask) > > > + if (event_mask & FAN_USER_OUTGOING_EVENTS & > > > + marks_mask & ~marks_ignored_mask) > > > return true; > > > > I don't like the _USER_ part of the constant name. How about _KNOWN_? > > I.e., FAN_KNOWN_OUTGOING_EVENTS sounds about like what it should? > > > > Do you mean just for *this* constant? I would rather have some uniformity > across constants if possible. No, I meant for all. Sorry for the confusion. > Errr... Maybe FAN_VALID_EVENTS/FAN_VALID_OUTGOING_EVENTS > Although _VALID_ is more appropriate for user inputs, so maybe just > FAN_KNOWN_OUTGOING_EVENTS as an exception to the convention > is fine. > > Or simply FAN_EVENTS_MASK/FAN_OUTGOING_EVENTS_MASK, like > the recent new constant FAN_MARK_TYPE_MASK? Yeah, MASK is fine with me as well. So are the names with VALID. Just pick one you like the most. > > > diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h > > > index 096c96f4f16a..a67430811006 100644 > > > --- a/include/linux/fanotify.h > > > +++ b/include/linux/fanotify.h > > > @@ -4,6 +4,56 @@ > > > > > > #include > > > > > > -/* not valid from userspace, only kernel internal */ > > > -#define FAN_MARK_ONDIR 0x00000100 > > > +/* > > > + * Flags not valid from userspace, only kernel internal. > > > + * Use high bits so we won't collide with userspace flags. > > > + */ > > > +#define FAN_MARK_ONDIR 0x80000000 > > > > This ought to be a separate change as I wrote above. > > > > Sure and maybe I'll name it FAN_KERN_MARK_ONDIR. Fine by me. Honza -- Jan Kara SUSE Labs, CR