[3/6] libtraceeval histogram: Have cmp and release functions be generic

Commit Message

Steven Rostedt Aug. 9, 2023, 3:13 a.m. UTC

From: "Steven Rostedt (Google)" <[e-mail]>

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.

In the structure, rename dyn_cmp to just cmp, and dyn_release to just
release.

Signed-off-by: Steven Rostedt (Google) <[e-mail]>
---
 include/traceeval-hist.h | 33 +++++++++++++++------------------
 src/histograms.c         | 26 ++++++++++++++------------
 2 files changed, 29 insertions(+), 30 deletions(-)

Patch

diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
index 03e050cdbed4..996fcab1b92b 100644
--- a/include/traceeval-hist.h
+++ b/include/traceeval-hist.h
@@ -65,13 +65,13 @@  struct traceeval_dynamic {
 struct traceeval_type;
 
 /* struct traceeval_dynamic release 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
@@ -79,8 +79,8 @@  typedef int (*traceeval_dyn_cmp_fn)(struct traceeval_dynamic *,
  * @name: The string name of the traceeval_data
  * @flags: flags to describe the traceeval_data
  * @id: User specified identifier
- * @dyn_release: For dynamic types called on release (ignored for other types)
- * @dyn_cmp: A way to compare dynamic types (ignored for other types)
+ * @data_release: An optional callback for when the data is being released
+ * @data_cmp: An optional callback to specify a way to compare the type
  *
  * The traceeval_type structure defines expectations for a corresponding
  * traceeval_data instance for a traceeval histogram instance. Used to
@@ -91,29 +91,26 @@  typedef int (*traceeval_dyn_cmp_fn)(struct traceeval_dynamic *,
  * which each relate to distinct user defined struct traceeval_dynamic
  * 'sub-types'.
  *
- * For flexibility, @dyn_cmp() and @dyn_release() take a struct
+ * For flexibility, @data_cmp() and @data_release() take a struct
  * traceeval_type instance. This allows the user to distinguish between
  * different sub-types of struct traceeval_dynamic within a single
  * callback function by examining the @id field. This is not a required
  * approach, merely one that is accommodated.
  *
- * @dyn_cmp() is used to compare two struct traceeval_dynamic instances when a
- * corresponding struct traceeval_type is reached with its type field set to
- * TRACEEVAL_TYPE_DYNAMIC. It should return 0 on equality, 1 if the first
- * argument is greater than the second, -1 for the other way around, and -2 on
- * error.
+ * @data_cmp() is used to override the default compare of a type. This is
+ * required to compare types of TRACEEVAL_TYPE_DYNAMIC. It should return 0
+ * on equality, 1 if the first argument is greater than the second,
+ * -1 for the other way around, and -2 on error.
  *
- * dyn_release() is used during traceeval_release() to release a union
- * traceeval_data's struct traceeval_dynamic field when the corresponding
- * traceeval_type type is set to TRACEEVAL_TYPE_DYNAMIC.
+ * data_release() is called when a data element is being released (or freed).
  */
 struct traceeval_type {
 	char				*name;
 	enum traceeval_data_type	type;
 	size_t				flags;
 	size_t				id;
-	traceeval_dyn_release_fn	dyn_release;
-	traceeval_dyn_cmp_fn		dyn_cmp;
+	traceeval_data_release_fn	release;
+	traceeval_data_cmp_fn		cmp;
 };
 
 /* Statistics about a given entry element */
diff --git a/src/histograms.c b/src/histograms.c
index a59542afdea1..f35fccb1d4fe 100644
--- a/src/histograms.c
+++ b/src/histograms.c
@@ -89,9 +89,9 @@  static int compare_traceeval_type(struct traceeval_type *orig,
 			return 1;
 		if (orig[i].id != copy[i].id)
 			return 1;
-		if (orig[i].dyn_release != copy[i].dyn_release)
+		if (orig[i].release != copy[i].release)
 			return 1;
-		if (orig[i].dyn_cmp != copy[i].dyn_cmp)
+		if (orig[i].cmp != copy[i].cmp)
 			return 1;
 
 		// make sure both names are same type
@@ -127,6 +127,9 @@  static int compare_traceeval_data(union traceeval_data *orig,
 	if (!copy)
 		return 1;
 
+	if (type->cmp)
+		return type->cmp(orig, copy, type);
+
 	switch (type->type) {
 	case TRACEEVAL_TYPE_STRING:
 		i = strcmp(orig->string, copy->string);
@@ -148,8 +151,7 @@  static int compare_traceeval_data(union traceeval_data *orig,
 		compare_numbers_return(orig->number_8, copy->number_8);
 
 	case TRACEEVAL_TYPE_DYNAMIC:
-		if (type->dyn_cmp)
-			return type->dyn_cmp(orig->dyn_data, copy->dyn_data, type);
+		/* If it didn't specify a cmp function, then punt */
 		return 0;
 
 	default:
@@ -235,8 +237,8 @@  static int compare_hist(struct traceeval *orig, struct traceeval *copy)
  *
  * This compares the values of the key definitions, value definitions, and
  * inserted data between @orig and @copy in order. It does not compare
- * by memory address, except for struct traceeval_type's dyn_release() and
- * dyn_cmp() fields.
+ * by memory address, except for struct traceeval_type's release() and
+ * cmp() fields.
  *
  * Returns 0 if @orig and @copy are the same, 1 if not, and -1 on error.
  */
@@ -450,14 +452,14 @@  static void clean_data(union traceeval_data *data, struct traceeval_type *defs,
 	}
 
 	for (i = 0; i < size; i++) {
+		if (defs[i].release) {
+			defs[i].release(defs + i, data + i);
+			continue;
+		}
 		switch (defs[i].type) {
 		case TRACEEVAL_TYPE_STRING:
 			free(data[i].string);
 			break;
-		case TRACEEVAL_TYPE_DYNAMIC:
-			if (defs[i].dyn_release)
-				defs[i].dyn_release(defs + i, data[i].dyn_data);
-			break;
 		default:
 			break;
 		}
@@ -506,7 +508,7 @@  static void hist_table_release(struct traceeval *teval)
  * it must call traceeval_release().
  *
  * This frees all internally allocated data of @teval and will call the
- * corresponding dyn_release() functions registered for keys and values of
+ * corresponding release() functions registered for keys and values of
  * type TRACEEVAL_TYPE_DYNAMIC.
  */
 void traceeval_release(struct traceeval *teval)
@@ -588,7 +590,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.
  */
 static void data_release(size_t size, union traceeval_data **data,
 				struct traceeval_type *type)