From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ipmail02.adl2.internode.on.net ([150.101.137.139]:9051 "EHLO ipmail02.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727434AbeJLOdU (ORCPT ); Fri, 12 Oct 2018 10:33:20 -0400 Date: Fri, 12 Oct 2018 18:01:53 +1100 From: Dave Chinner Subject: Re: [PATCH 2/2] xfs: use offsetof() in place of offset macros for __xfsstats Message-ID: <20181012070153.GW6311@dastard> References: <20181010123708.7632-1-cmaiolino@redhat.com> <20181010123708.7632-3-cmaiolino@redhat.com> <20181011052957.GO6311@dastard> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Eric Sandeen Cc: Carlos Maiolino , linux-xfs@vger.kernel.org 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 ..... > > 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. 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