Re: [PATCH 3/5] xfs: refactor repair forcing tests into a repair.c helper

Christoph Hellwig
2023-11-29T06:03:59Z

On Tue, Nov 28, 2023 at 09:42:01PM -0800, Darrick J. Wong wrote:
> > I find this code organization where the check helpers are in foo.c,
> > repair helpers in foo_repair.c and then both are used in scrub.c
> > to fill out ops really annoying to follow. My normal taste would
> > expect a single file that has all the methods, and which then
> > registers the ops vector. But it's probably too late for that now..
>
> Not really, in theory I could respin the whole series to move
> FOO_repair.c into FOO.c surrounded by a giant #ifdef
> CONFIG_XFS_ONLINE_REPAIR block; and change agheader_repair.c.
>
> OTOH I thought it was cleaner to elide the repair code via Makefiles
> instead of preprocessor directives that we could get lost in.
>
> Longer term I've struggled with whether or not (for example) the
> alloc.c/alloc_repair.c declarations should go in alloc.h. That's
> cleaner IMHO but explodes the number of files for usually not that much
> gain.

Heh, and I wondered if the check/repair code should just live with
the code implenenting the functionality it is checking/repairing so
things can be kept nicely static. I really do not have a good answer
here, I just noticed that it requires quite a lot of cycling through
files to understand the repair code.