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.