From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 25 Feb 2020 20:35:39 GMT
Subject: Re: [PATCH v5 2/3] advice: revamp advise API
Message-ID: <xmqq36aymmlg.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <xmqqblpmmntj.fsf@gitster-ct.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

> Emily Shaffer <emilyshaffer@google.com> writes:
>
>>> ...  Your get_config_value() would then become a mere lookup
>>> in advice_setting[] array, e.g.
>>> 
>>> 	int advice_enabled(unsigned advice_type)
>>> 	{
>>> 		static int initialized;
>>> 
>>> 		if (!initialized) {
>>> 			initialized = 1;
>>> 			git_config(populate_advice_settings, NULL);
>>> 		}
>>> 		if (ARRAY_SIZE(advice_setting) <= advice_type)
>>> 			BUG("OOB advice type requested???");
>>> 		return !advice_setting[advice_type].disabled;
>>> 	}
>>> 
>>> with your "push-update-rejected has two names" twist added.

One beauty of the approach is that the "twist" can be done in the
initialization codepath, e.g.

 	int advice_enabled(unsigned advice_type)
 	{
 		static int initialized;
 
 		if (!initialized) {
 			initialized = 1;
 			git_config(populate_advice_settings, NULL);

                        advice_setting[ADVICE_PUSH_UPDATE_REJECTED] &=
                        advice_setting[ADVICE_PUSH_UPDATE_REJECTED_ALIAS];
 		}
 		if (ARRAY_SIZE(advice_setting) <= advice_type)
 			BUG("OOB advice type requested???");
 		return !advice_setting[advice_type].disabled;
 	}

which means that the function literally becomes an array access that
is guarded for out-of-bounds index.

Thanks, Emily, for making me look at the suggested code again to
realize this ;-)