On Tue, Aug 08, 2023 at 12:11:58PM -0400, Stevie Alvarez wrote:
> From: Stevie Alvarez (Google) <stevie.6strings@xxxxxxxxx>
>
> traceeval_insert() updates the provided struct traceeval's histogram.
> If an entry that exists with a keys field that match the keys argument,
> the entries vals field are updated with a copy of the vals argument.
> If such an entry does not exist, a new entry is added to the histogram,
> with respect to the keys and vals arguments.
>
> Signed-off-by: Stevie Alvarez (Google) <stevie.6strings@xxxxxxxxx>
> ---
> include/traceeval-hist.h | 4 ++
> src/histograms.c | 106 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 110 insertions(+)
>
> diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
> index 4923de1..e713c70 100644
> --- a/include/traceeval-hist.h
> +++ b/include/traceeval-hist.h
> @@ -134,6 +134,10 @@ struct traceeval *traceeval_init(const struct traceeval_type *keys,
>
> void traceeval_release(struct traceeval *teval);
>
> +int traceeval_insert(struct traceeval *teval,
> + const union traceeval_data *keys,
> + const union traceeval_data *vals);
> +
> int traceeval_query(struct traceeval *teval, const union traceeval_data *keys,
> union traceeval_data **results);
>
> diff --git a/src/histograms.c b/src/histograms.c
> index 47ff175..a59542a 100644
> --- a/src/histograms.c
> +++ b/src/histograms.c
> @@ -684,3 +684,109 @@ void traceeval_results_release(struct traceeval *teval,
>
> data_release(teval->nr_val_types, &results, teval->val_types);
> }
> +
> +/*
> + * Create a new entry in @teval with respect to @keys and @vals.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +static int create_entry(struct traceeval *teval,
> + const union traceeval_data *keys,
> + const union traceeval_data *vals)
> +{
> + union traceeval_data *new_keys;
> + union traceeval_data *new_vals;
> + struct entry *tmp_map;
> + struct hist_table *hist = teval->hist;
> +
> + /* copy keys */
> + if (copy_traceeval_data_set(teval->nr_key_types, teval->key_types,
> + keys, &new_keys) == -1)
> + return -1;
> +
> + /* copy vals */
> + if (copy_traceeval_data_set(teval->nr_val_types, teval->val_types,
> + vals, &new_vals) == -1)
> + goto fail_vals;
> +
> + /* create new entry */
> + tmp_map = realloc(hist->map, ++hist->nr_entries * sizeof(*tmp_map));
> + if (!tmp_map)
> + goto fail;
> + tmp_map->keys = new_keys;
> + tmp_map->vals = new_vals;
> + hist->map = tmp_map;
> +
> + return 0;
> +
> +fail:
> + data_release(teval->nr_val_types, &new_vals, teval->val_types);
> +
> +fail_vals:
> + data_release(teval->nr_key_types, &new_keys, teval->key_types);
> + return -1;
> +}
> +
> +/*
> + * Update @entry's vals field with a copy of @vals, with respect to @teval.
> + *
> + * Return 0 on success, -1 on error.
> + */
> +static int update_entry(struct traceeval *teval, struct entry *entry,
> + const union traceeval_data *vals)
> +{
> + union traceeval_data *new_vals;
> +
> + if (copy_traceeval_data_set(teval->nr_val_types, teval->val_types,
> + vals, &new_vals) == -1)
> + return -1;
> +
> + entry->vals = new_vals;
Are we leaking the old 'entry->vals', as we never free that memory anywhere?
Should we just update in-place, instead of allocating a new one & freeing the
old? The vals arrays should each be the same size because they have a
common 'teval->val_types'.
I know 'vals' is owned by the caller, but we allocated and own 'entry->vals'
via a previous call to create_entry().
We'll have to free existing strings & allocate new ones (and do something
similar for dynamic types when they're supported), but the rest of
'entry->vals' should be reusable I think.
> + return 0;
> +}
> +
> +/*
> + * traceeval_insert - insert an item into the traceeval descriptor
> + * @teval: The descriptor to insert into
> + * @keys: The list of keys that defines what is being inserted.
> + * @vals: The list of values that defines what is being inserted.
> + *
> + * The @keys is an array that holds the data in the order of the keys
> + * passed into traceeval_init(). That is, if traceeval_init() had
> + * keys = { { .type = TRACEEVAL_STRING }, { .type = TRACEEVAL_NUMBER_8 },
> + * { .type = TRACEEVAL_NONE } }; then the @keys array passed in must
> + * be a string (char *) followed by a 8 byte number (char).
> + *
> + * The @keys and @vals are only examined to where it expects data. That is,
> + * if the traceeval_init() keys had 3 items where the first two was defining
> + * data, and the last one was the TRACEEVAL_TYPE_NONE, then the @keys
> + * here only needs to be an array of 2, inserting the two items defined
> + * by traceeval_init(). The same goes for @vals.
> + *
> + * For all elements of @keys and @vals that correspond to a struct
> + * traceeval_type of type TRACEEVAL_TYPE_STRING, the string field must be set
> + * a valid pointer or NULL.
> + *
> + * On error, @teval is left unchanged.
> + *
> + * Returns 0 on success, and -1 on error.
> + */
> +int traceeval_insert(struct traceeval *teval,
> + const union traceeval_data *keys,
> + const union traceeval_data *vals)
> +{
> + struct entry *entry;
> + int check;
> +
> + entry = NULL;
> + check = get_entry(teval, keys, &entry);
> +
> + if (check == -1)
> + return check;
> +
> + /* insert key-value pair */
> + if (check)
> + return create_entry(teval, keys, vals);
> + else
> + return update_entry(teval, entry, vals);
> +}
> --
> 2.41.0
>
|