On Fri, Aug 11, 2023 at 01:39:28AM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
>
> When looking at how this code would be implemented, I found that I needed
> access to the traceeval structure within the compare callbacks. Pass that
> in too.
>
> Also, rearrange the parameters a little. The traceeval and type should go
> first as they describe the "object", and the data should go last as they are
> the values of the function.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
> include/traceeval-hist.h | 13 +++++++------
> src/histograms.c | 19 ++++++++++---------
> 2 files changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
> index 22e9029133d5..412efdbe8681 100644
> --- a/include/traceeval-hist.h
> +++ b/include/traceeval-hist.h
> @@ -63,16 +63,17 @@ union traceeval_data {
> };
>
> struct traceeval_type;
> +struct traceeval;
>
> /* struct traceeval_dynamic release function signature */
> -typedef void (*traceeval_data_release_fn)(struct traceeval_type *,
> - union traceeval_data *);
> +typedef void (*traceeval_data_release_fn)(const struct traceeval_type *type,
> + union traceeval_data *data);
>
> /* struct traceeval_dynamic compare function signature */
> -typedef int (*traceeval_data_cmp_fn)(const union traceeval_data *,
> - const union traceeval_data *,
> - struct traceeval_type *);
> -
> +typedef int (*traceeval_data_cmp_fn)(struct traceeval *teval,
> + const struct traceeval_type *type,
> + const union traceeval_data *A,
> + const union traceeval_data *B);
> /*
> * struct traceeval_type - Describes the type of a traceevent_data instance
> * @type: The enum type that describes the traceeval_data
> diff --git a/src/histograms.c b/src/histograms.c
> index 09cdf57a8f6a..ece1395ac300 100644
> --- a/src/histograms.c
> +++ b/src/histograms.c
> @@ -113,7 +113,8 @@ static int compare_traceeval_type(struct traceeval_type *orig,
> * Return 0 if @orig and @copy are the same, 1 if @orig is greater than @copy,
> * -1 for the other way around, and -2 on error.
> */
> -static int compare_traceeval_data(union traceeval_data *orig,
> +static int compare_traceeval_data(struct traceeval *teval,
> + union traceeval_data *orig,
> const union traceeval_data *copy,
> struct traceeval_type *type)
Nit: it would be nice if these had the same ordering as the args to
traceeval_data_cmp_fn.
Other than that you can add:
Reviewed-by: Ross Zwisler <zwisler@xxxxxxxxxx>
|