On Fri, Jul 28, 2023 at 03:04:38PM -0400, Stevie Alvarez wrote:
> From: "Stevie Alvarez (Google)" <stevie.6strings@xxxxxxxxx>
>
> traceeval_release() deconstructs a given struct traceeval instance. It
> frees any data allocated to the heap within the union traceeval_data
> arrays of entries to the histogram, and the names allocated for the
> struct traceeval_type key-value definitions.
>
> Signed-off-by: Stevie Alvarez (Google) <stevie.6strings@xxxxxxxxx>
> ---
> src/histograms.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 85 insertions(+), 2 deletions(-)
>
> diff --git a/src/histograms.c b/src/histograms.c
> index 13830e4..f46a0e0 100644
> --- a/src/histograms.c
> +++ b/src/histograms.c
> @@ -209,10 +209,93 @@ fail_eval_init_unalloced:
> return NULL;
> }
>
> -// TODO
> -void traceeval_release(struct traceeval *teval)
> +/**
> + * Deallocate array of traceeval_type's, which must be terminated by
> + * TRACEEVAL_TYPE_NONE.
> + */
> +static void type_release(struct traceeval_type *defs)
> {
> + size_t i = 0;
> +
> + if (!defs)
> + return;
> +
> + for_each_key(i, defs) {
> + if (defs[i].name)
> + free(defs[i].name);
> + }
> +
> + free(defs);
> +}
> +
> +/**
> + * Deallocate any specified dynamic data in @data.
> + */
> +static void clean_data(union traceeval_data *data, struct traceeval_type *def)
> +{
> + size_t i = 0;
> +
> + if (!data || !def)
> + return;
> +
> + for_each_key(i, def) {
> + switch (def[i].type) {
> + case TRACEEVAL_TYPE_STRING:
> + if (data[i].string)
> + free(data[i].string);
> + break;
> + case TRACEEVAL_TYPE_DYNAMIC:
> + def[i].dyn_release(data[i].dyn_data, &def[i]);
> + break;
> + default:
> + break;
> + }
> + }
> +}
>
> +/**
> + * Deallocate all possible data stored within the entry.
> + */
> +static void clean_entry(struct entry *entry, struct traceeval *teval)
> +{
> + if (!entry)
Should we check for NULL 'teval' as well?
> + return;
> +
> + // deallocate dynamic traceeval_data
> + clean_data(entry->keys, teval->def_keys);
> + clean_data(entry->vals, teval->def_vals);
> + free(entry->keys);
> + free(entry->vals);
> +}
> +
> +/**
> + * Deallocate the hist_table allocated to a traceeval instance.
> + */
> +static void hist_table_release(struct traceeval *teval)
> +{
> + struct hist_table *hist = teval->hist;
> +
> + if (!hist)
> + return;
> +
> + for (size_t i = 0; i < hist->nr_entries; i++) {
> + clean_entry(&hist->map[i], teval);
> + }
> + free(hist->map);
> + free(hist);
Probably good to set 'teval->hist = NULL' just for good hygiene. This keeps
later users from accidentally having a use-after-free.
|