Re: [PATCH 2/2] xfs: use offsetof() in place of offset macros for __xfsstats

[Dave Chinner <david@fromorbit.com>]

On Thu, Oct 11, 2018 at 09:04:14AM -0500, Eric Sandeen wrote:
> On 10/11/18 12:29 AM, Dave Chinner wrote:
> > On Wed, Oct 10, 2018 at 02:37:08PM +0200, Carlos Maiolino wrote:
> >> Most offset macro mess is used in xfs_stats_format() only, and we can
> >> simply get the right offsets using offsetof(), instead of several macros
> >> to mark the offsets inside __xfsstats structure.
> >>
> >> Replace all XFSSTAT_END_* macros by a single helper macro to get the
> >> right offset into __xfsstats, and use this helper in xfs_stats_format()
> >> directly.
> >>
> >> The quota stats code, still looks a bit cleaner when using XFSSTAT_*
> >> macros, so, this patch also defines XFSSTAT_START_XQMSTAT and
> >> XFSSTAT_END_XQMSTAT locally to that code. This also should prevent
> >> offset mistakes when updates are done into __xfsstats.
> >>
> >> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
.....
> > Now that I look at this more closely , it's a little bit ... odd.
> > i.e. the endpoint offset points to the first variable in the next
> > line, not the last variable of the stats line described by the text.
> > While it works, it just doesn't seem quite right to me.
....
> and "what can go wrong" is somebody extends one of the groups without
> changing the end, or they insert a group without changing where the xqm
> stats start.

<snip>

I don't really care that much - if was easy to change it would be
nice, but wasting more than a couple of minutes on it isn't worth
anyone's time. It's not complex, nor is it critical code so we do
not need to bikeshed this to death. I'll just take the original
patch as it's just a replacement of existing code and so is are
acceptable.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com