| Message ID | 20230811053940.1408424-6-rostedt@goodmis.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libtraceeval histogram: Updates | expand |
On Fri, Aug 11, 2023 at 01:39:28AM -0400, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > 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@goodmis.org> > --- > 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@google.com>
On Tue, 15 Aug 2023 10:55:54 -0600 Ross Zwisler <zwisler@google.com> wrote: > > --- 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. > Yeah, I noticed that to, but figured I can change it later, as it's not API. I'll update it. > Other than that you can add: > > Reviewed-by: Ross Zwisler <zwisler@google.com> Thanks Ross, -- Steve
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) { @@ -129,7 +130,7 @@ static int compare_traceeval_data(union traceeval_data *orig, return 1; if (type->cmp) - return type->cmp(orig, copy, type); + return type->cmp(teval, type, orig, copy); switch (type->type) { case TRACEEVAL_TYPE_STRING: @@ -167,7 +168,8 @@ static int compare_traceeval_data(union traceeval_data *orig, * * Return 1 if @orig and @copy are the same, 0 if not, and -1 on error. */ -static int compare_traceeval_data_set(union traceeval_data *orig, +static int compare_traceeval_data_set(struct traceeval *teval, + union traceeval_data *orig, const union traceeval_data *copy, struct traceeval_type *defs, size_t size) { @@ -176,7 +178,7 @@ static int compare_traceeval_data_set(union traceeval_data *orig, /* compare data arrays */ for (i = 0; i < size; i++) { - if ((check = compare_traceeval_data(orig + i, copy + i, defs + i))) + if ((check = compare_traceeval_data(teval, orig + i, copy + i, defs + i))) return check == -2 ? -1 : 0; } @@ -192,13 +194,13 @@ static int compare_entries(struct entry *orig, struct entry *copy, int check; /* compare keys */ - check = compare_traceeval_data_set(orig->keys, copy->keys, + check = compare_traceeval_data_set(teval, orig->keys, copy->keys, teval->key_types, teval->nr_key_types); if (check < 1) return check; /* compare values */ - check = compare_traceeval_data_set(orig->vals, copy->vals, + check = compare_traceeval_data_set(teval, orig->vals, copy->vals, teval->val_types, teval->nr_val_types); return check; } @@ -546,9 +548,8 @@ static int get_entry(struct traceeval *teval, const union traceeval_data *keys, hist = teval->hist; for (i = 0, entry = hist->map; i < hist->nr_entries; entry = &hist->map[++i]) { - check = compare_traceeval_data_set(entry->keys, keys, - teval->key_types, teval->nr_key_types); - + check = compare_traceeval_data_set(teval, entry->keys, keys, + teval->key_types, teval->nr_key_types); if (!check) continue; break;