On Tue, Nov 07, 2023 at 05:01:16PM +0900, Michael Paquier wrote:
> On Mon, Nov 06, 2023 at 10:28:14PM +0300, Nazir Bilal Yavuz wrote:
>> I liked the idea; thanks for working on this!
+1, this seems very useful.
> +#ifdef USE_INJECTION_POINTS
> +#define INJECTION_POINT_RUN(name) InjectionPointRun(name)
> +#else
> +#define INJECTION_POINT_RUN(name) ((void) name)
> +#endif
nitpick: Why is the non-injection-point version "(void) name"? I see
"(void) true" used elsewhere for this purpose.
> + <para>
> + Here is an example of callback for
> + <literal>InjectionPointCallback</literal>:
> +<programlisting>
> +static void
> +custom_injection_callback(const char *name)
> +{
> + elog(NOTICE, "%s: executed custom callback", name);
> +}
> +</programlisting>
Why do we provide the name to the callback functions?
Overall, the design looks pretty good to me. I think it's a good idea to
keep it simple to start with. Since this is really only intended for
special tests that run in special builds, it seems like we ought to be able
to change it easily in the future as needed.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
