Glen Choo <chooglen@xxxxxxxxxx> writes: > I've covered most your response to Ævar upthread, so I'll omit that. Thanks. Indeed, I missed the situation in which a caller used kvi not by accessing its fields directly but by passing kvi to a function in config.c. > In an off-list discussion, we described some plausible ways to organize > the refactor that would make it easier for a reviewer to confirm safety. > > I haven't tried that yet because it sounds like you'd prefer the > sidestepping approach. Do you prefer that primarily for safety reasons, > or is it largely motivated by other concerns too (e.g. reducing churn or > sidestepping produces a better API)? If the primary concern is just > safety, I'm somewhat confident that we can find some way to organize > this that makes it easier to review and I should just do it. My preference for the sidestepping approach was to reduce churn, but as you have pointed out, it doesn't actually reduce churn. So now I think that the patches should be reordered (but am open to being convinced otherwise, of course). As for safety and better API, I think both approaches (bulk modification of all functions to take the new config_fn_t and two sets of functions each taking a different function type) are equally safe, and it is bulk modification that results in a better API (as you've demonstrated, having kvi information is needed for good error messages, and I expect that to be more and more needed).