Re: [PATCH 0/7] fix segfaults with implicit-bool config

From: Jeff King <peff@peff.net>
Date: Dec 12, 2023, 00:52 UTC
Message-ID: <20231212005228.GB376323@coredump.intra.peff.net>
In-Reply-To: <ZXF-7AMZ_SBltplk@tanuki>

On Thu, Dec 07, 2023 at 09:14:36AM +0100, Patrick Steinhardt wrote:

> Thanks for working on this topic! I've left a couple of comments, most
> of which are about whether we should retain previous behaviour where we
> generate a warning instead of raising an error for unknown values.

Thanks for taking a look. I see what you're saying about the warnings, but IMHO it's not worth the extra complexity. Returning early means the existing code can proceed without worrying about NULLs. Though I suppose we could have a "warn_error_nonbool()" which issues a warning and returns 0.

Still, the "return config_error_nonbool()" pattern is pretty well-established and used for most options. I would go so far as to say the ones that warn for invalid values are the odd ones out, and probably should be returning errors as well (though doing so now may not be worth the trouble and risk of annoyance).

And certainly there should be no regressions in this series; every case is currently a segfault, so returning an error is a strict improvement. I'd just as soon stay strict there, as it's easier to loosen later if we choose than to tighten.

-Peff