Thank you for this enhancement to injection points.
Following are a few comments.
+ * This overestimates the allocation size, including slots that may be + * free, for simplicity. + */ + result = palloc0(sizeof(InjectionPointData) * max_inuse); +
The result variable name is a bit generic. How about renaming it to inj_points/injpnt_list or something similar?
+/* + * Retrieve a list of all the injection points currently attached. + * + * This list is palloc'd in the current memory context. + */ +InjectionPointData * +InjectionPointList(uint32 *num_points)
The function actually retrieves an array not a list, so the comment and the function name may be misleading.
This function not only retrieves an array of injection points but also
the number of injection points. Would it be more efficient to define a single struct that includes both the array of injection points and the number of points? This way, both values can be returned from the function.
The second patch is still something folks seem to be meh about, but if the end consensus is to put the new SQL function in the module injection_points we are going to need the first patch anyway.
FWIW, I believe it would be beneficial to incorporate the new SQL function into the core. This would eliminate the need for users to install the injection_points module solely to monitor injection points attached in the
other modules or within the core itself.
+Datum +pg_get_injection_points(PG_FUNCTION_ARGS)
Would it be useful to put the logic of the above function under #define USE_INJECTION_POINT. This approach would make it possible to distinguish between cases where no injection points are attached and instances where the build does not support injection points.