On Fri, Aug 11, 2023 at 01:39:27AM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
>
> Having the ability to override the compare function for a given type can
> be very advantageous. There's also no reason that any type could ask for a
> release callback to be called when the type is being released. It could be
> used for information as well as for freeing.
>
> Rename traceeval_dyn_cmp_fn to traceeval_data_cmp_fn and
> traceeval_dyn_release_fn to traceeval_data_release_fn and have
> them take the union traceeval_type instead of struct traceeval_dynamic.
>
> Also this changes the compare to pass a pointer to union traceeval_data
> instead of passing in the structure of the dyn_data type.
>
> In the structure, rename dyn_cmp to just cmp, and dyn_release to just
> release.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
> include/traceeval-hist.h | 46 +++++++++++++++++++++-------------------
> src/histograms.c | 28 ++++++++++++------------
> 2 files changed, 38 insertions(+), 36 deletions(-)
>
> diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
> index f6c4e8efb2be..22e9029133d5 100644
> --- a/include/traceeval-hist.h
> +++ b/include/traceeval-hist.h
> @@ -65,13 +65,13 @@ union traceeval_data {
> struct traceeval_type;
>
> /* struct traceeval_dynamic release function signature */
You may want to update the comment here to make it clear that this is now for
both dynamic and pointer types. Ditto for the compare function signature.
> -typedef void (*traceeval_dyn_release_fn)(struct traceeval_type *,
> - struct traceeval_dynamic);
> +typedef void (*traceeval_data_release_fn)(struct traceeval_type *,
> + union traceeval_data *);
>
> /* struct traceeval_dynamic compare function signature */
> -typedef int (*traceeval_dyn_cmp_fn)(struct traceeval_dynamic,
> - struct traceeval_dynamic,
> - struct traceeval_type *);
> +typedef int (*traceeval_data_cmp_fn)(const union traceeval_data *,
> + const union traceeval_data *,
> + struct traceeval_type *);
>
> /*
> * struct traceeval_type - Describes the type of a traceevent_data instance
<>
> @@ -588,7 +588,7 @@ static int copy_traceeval_data(struct traceeval_type *type,
> /*
> * Free @data with respect to @size and @type.
> *
> - * Does not call dyn_release on type TRACEEVAL_TYPE_DYNAMIC.
> + * Does not call release on type TRACEEVAL_TYPE_DYNAMIC.
or for TRACEEVAL_TYPE_POINTER.
> */
> static void data_release(size_t size, union traceeval_data **data,
> struct traceeval_type *type)
> --
> 2.40.1
Aside from these two comment nits, you can add:
Reviewed-by: Ross Zwisler <zwisler@xxxxxxxxxx>
|