On Thu, Nov 21, 2024 at 12:16 PM Jan Kara <jack@xxxxxxx> wrote: > > On Thu 21-11-24 12:04:23, Amir Goldstein wrote: > > On Thu, Nov 21, 2024 at 11:09 AM Christian Brauner <brauner@xxxxxxxxxx> wrote: > > > > > > > It is not that I object to "two bit constants". FMODE_FSNOTIFY_MASK is a > > > > two-bit constant and a good one. But the name clearly suggests it is not a > > > > single bit constant. When you have all FMODE_FOO and FMODE_BAR things > > > > single bit except for FMODE_BAZ which is multi-bit, then this is IMHO a > > > > recipe for problems and I rather prefer explicitely spelling the > > > > combination out as FMODE_NONOTIFY | FMODE_NONOTIFY_PERM in the few places > > > > that need this instead of hiding it behind some other name. > > > > > > Very much agreed! > > > > Yes, I agree as well. > > What I meant is that the code that does > > return FMODE_NONOTIFY | FMODE_NONOTIFY_PERM; > > > > is going to be unclear to the future code reviewer unless there is > > a comment above explaining that this is a special flag combination > > to specify "suppress only pre-content events". > > So this combination is used in file_set_fsnotify_mode() only (three > occurences) and there I have: > > /* > * If there are permission event watchers but no pre-content event > * watchers, set FMODE_NONOTIFY | FMODE_NONOTIFY_PERM to indicate that. > */ > > at the first occurence. So hopefully that's enough of an explanation. > Yes, that's the comment that I did not see, but assumed it was there ;) which I wrongly expressed as "I wonder how you annotated". Thanks, Amir.
