Re: [PATCHv2 12/13] credentials: add "store" helper

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 6, 2011, 21:50 UTC
Message-ID: <7vzkf5e5dp.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20111206062305.GL29233@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 14 quoted lines

> +static void store_credential(const char *fn, struct credential *c)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +
> +	/*
> +	 * Sanity check that what we are storing is actually sensible.
> +	 * In particular, we can't make a URL without a protocol field.
> +	 * Without either a host or pathname (depending on the scheme),
> +	 * we have no primary key. And without a username and password,
> +	 * we are not actually storing a credential.
> +	 */
> +	if (!c->protocol || !(c->host || c->path) ||
> +	    !c->username || !c->password)
> +		return;

Very nicely explained. I wish all our patches had comments like this to explain tricky bit that looks as if the choice was arbitrarily made but in fact the logic was carefully constructed.

Thanks.