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 */ -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) + * @release: An optional callback for when the data is being released + * @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,31 @@ 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 - * 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. + * For flexibility, @cmp() and @release() take a struct traceeval_type + * instance. This allows the user to handle dyn_data and pointer types. + * It may also be used for other types if the default cmp() or release() + * need to be overridden. Note for string types, even if the release() + * is called, the string freeing is still taken care of by the traceeval + * infrastructure. * - * @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. + * The @id field is a user specified field that may allow the same callback + * to be used by multiple types and not needing to do a strcmp() against the + * name (could be used for switch statements). * - * 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. + * @cmp() is used to override the default compare of a type. This is + * required to compare dyn_data and pointer types. 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. + * + * @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 d22d15238616..09cdf57a8f6a 100644 --- a/src/histograms.c +++ b/src/histograms.c @@ -90,9 +90,9 @@ static int compare_traceeval_type(struct traceeval_type *orig, return 0; if (orig[i].id != copy[i].id) return 0; - if (orig[i].dyn_release != copy[i].dyn_release) + if (orig[i].release != copy[i].release) return 0; - if (orig[i].dyn_cmp != copy[i].dyn_cmp) + if (orig[i].cmp != copy[i].cmp) return 0; // make sure both names are same type @@ -128,6 +128,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); @@ -149,8 +152,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: @@ -236,8 +238,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 1 if @orig and @copy are the same, 0 if not, and -1 on error. */ @@ -438,14 +440,13 @@ fail: */ static void clean_data(union traceeval_data *data, struct traceeval_type *type) { + if (type->release) + type->release(type, data); + switch (type->type) { case TRACEEVAL_TYPE_STRING: free(data->string); break; - case TRACEEVAL_TYPE_DYNAMIC: - if (type->dyn_release) - type->dyn_release(type, data->dyn_data); - break; default: break; } @@ -465,9 +466,8 @@ static void clean_data_set(union traceeval_data *data, struct traceeval_type *de return; } - for (i = 0; i < size; i++) { + for (i = 0; i < size; i++) clean_data(data + i, defs + i); - } free(data); } @@ -512,7 +512,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 +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. */ static void data_release(size_t size, union traceeval_data **data, struct traceeval_type *type) -- 2.40.1