Re: [PATCH v6 1/4] fs: move sgid strip operation from inode_init_owner into inode_sgid_strip

[Matthew Wilcox <willy@infradead.org>]

On Mon, Apr 25, 2022 at 01:29:47PM +0200, Christian Brauner wrote:
> On Mon, Apr 25, 2022 at 03:08:36AM +0000, xuyang2018.jy@fujitsu.com wrote:
> > on 2022/4/25 10:45, Matthew Wilcox wrote:
> > > On Mon, Apr 25, 2022 at 11:09:38AM +0800, Yang Xu wrote:
> > >> This has no functional change. Just create and export inode_sgid_strip
> > >> api for the subsequent patch. This function is used to strip inode's
> > >> S_ISGID mode when init a new inode.
> > >
> > > Why would you call this inode_sgid_strip() instead of
> > > inode_strip_sgid()?
> > 
> > Because I treated "inode sgid(inode's sgid)" as a whole.
> > 
> > inode_strip_sgid sounds also ok, but now seems strip_inode_sgid seem 
> > more clear because we strip inode sgid depend on not only inode's 
> > condition but also depend on parent directory's condition.
> > 
> > What do you think about this?
> > 
> > ps: I can aceept the above several way, so if you insist, I can change 
> > it to inode_strip_sgid.
> 
> I agree with Willy. I think inode_strip_sgid() is better. It'll be in
> good company as <object>_<verb>_<what?> is pretty common:
> 
> inode_update_atime()
> inode_init_once()
> inode_init_owner()
> inode_init_early()
> inode_add_lru()
> inode_needs_sync()
> inode_set_flags()
> 
> Maybe mode_remove_sgid() is even better because it makes it clear that
> the change happens to @mode and not @dir. But I'm fine with
> inode_strip_sgid() or inode_remove_sgid() too.

Oh!  Yes, mode_strip_sgid() is better.  We're operating on the mode,
not the inode.