Re: [PATCH v2] credential: clear expired c->credential, unify secret clearing

From: Jeff King <peff@peff.net>
Date: Jun 6, 2024, 08:10 UTC
Message-ID: <20240606081054.GC658959@coredump.intra.peff.net>
In-Reply-To: <xmqqtti7tj32.fsf@gitster.g>

On Wed, Jun 05, 2024 at 10:06:41AM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> So we have clear() that is to "clear everything", clear_secret()
> that is to "clear auth material", but we would want another "clear
> every members other than used as query keys" level?
> 
> That way, anytime we add different kind of "auth material" (like
> brian's series did), existing code paths that call clear_secret() do
> not have to change, and if we add different kind of "query keys",
> the reject code would not have to change?  Or is the reject code
> path the only thing that cares about what members are used as query
> keys, in which case we do not need the third helper?

I can't think of another place besides the reject path where we'd want that (though I'm certainly open to being corrected if somebody finds such a spot). But mostly I am not all that confident that the set of items that reject() is clearing is the best one. So I'd just as soon leave it as a weird internal detail for now, rather than codifying it in a function.

I dunno. I guess it is the same lines of code in either spot, but somehow sticking it in a clear_response() helper seems like an endorsement that the author knew what they were doing. ;)

-Peff