From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 12 Dec 2023 04:10:01 GMT
Subject: Re: [PATCH 0/7] fix segfaults with implicit-bool config
Message-ID: <ZXfdGYUyJz4bnkQ0@tanuki>
In-Reply-To: <20231212005228.GB376323@coredump.intra.peff.net>

On Mon, Dec 11, 2023 at 07:52:28PM -0500, Jeff King wrote:
> 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.

Fair enough, I'm perfectly fine with this reasoning. Thanks!

Patrick