Re: [PATCH 1/5] xfs: convert XFS_IFORK_PTR to a static inline helper

[Dave Chinner <david@fromorbit.com>]

On Mon, Jul 11, 2022 at 09:53:29PM -0700, Christoph Hellwig wrote:
> On Mon, Jul 11, 2022 at 06:20:40PM -0700, Darrick J. Wong wrote:
> > I personally am not that bothered by shouty function names, but Dave
> > has asked for shout-reduction in the past, so every time I convert
> > something I also change the case.
> > 
> > AFAIK it /is/ sort of a C custom that macros get loud names and
> > functions do not so that you ALWAYS KNOW, erm, when you're dealing with
> > a macro that could rain bad coding conventions down on your head.
> 
> It is a bit of a historic custom, but not really followed strictly.
> I tend to think of trivial container_of and similar addressing inline
> functions just as macros with a saner implementation, and so does a
> big part of the kernel community.  Where exactly that border is is not
> clear, though.  And in doubt I think avoiding too much churn in changing
> things unless there is a clear benefit is a good idea.  So maybe we
> would have picked a lower case name for XFS_IFORK_PTR when adding it
> new, but i don't really see any benefit in changing it now.

/me shrugs

All I really want is the macros converted to static inlines so that
we get proper type checking. I'm not concerned by upper/lower case
that much, and for stuff like XFS_I/VFS_I I agree that it makes
sense because they are really short and it makes the type
conversions of related embedded objects stand out in the code
nicely.

But for longer stuff like xfs_ifork_ptr(), I think it makes more
sense to follow the "UPPER == macro, lower == static inline"
conventions, especially in the dense forest of similar macro
definitions surrounding XFS_IFORK_... and XFS_DFORK...

Yes, it does mean there's a little bit of eye retraining to be done
for long term developers, but I think the end result is much more
easier to understand especially for people new to the XFS code....

> The same is true for some of the other patches later in the series,
> except for maybe XFS_IFORK_Q, which has been really grossly and
> confusingly misnamed.

One of many. :/

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com