Re: [PATCH v3 18/18] libtraceeval: Add traceeval_remove()

[Ross Zwisler <zwisler@google.com>]

On Wed, Aug 16, 2023 at 09:33:10PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> Add a way to remove an entry that's in the traceeval. Currently there's
> only ways to add entries, but there also needs to be a way to remove them
> too.
> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  include/traceeval-hist.h |  3 +++
>  src/histograms.c         | 29 +++++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
> index 37147d626fbb..8e5a6451f399 100644
> --- a/include/traceeval-hist.h
> +++ b/include/traceeval-hist.h
> @@ -160,6 +160,9 @@ int traceeval_insert(struct traceeval *teval,
>  		     const union traceeval_data *keys,
>  		     const union traceeval_data *vals);
>  
> +int traceeval_remove(struct traceeval *teval,
> +		     const union traceeval_data *keys);
> +
>  int traceeval_query(struct traceeval *teval, const union traceeval_data *keys,
>  		    const union traceeval_data **results);
>  
> diff --git a/src/histograms.c b/src/histograms.c
> index c16b8866b84a..f35d1b2e583d 100644
> --- a/src/histograms.c
> +++ b/src/histograms.c
> @@ -934,6 +934,35 @@ int traceeval_insert(struct traceeval *teval,
>  		return update_entry(teval, entry, vals);
>  }
>  
> +/**
> + * traceeval_remove - remove an item from the traceeval descriptor
> + * @teval: The descriptor to insert into
                                remove from

Aside from that one nit you can add:

Reviewed-by: Ross Zwisler <zwisler@google.com>

> + * @keys: The list of keys that defines what is being removed
> + *
> + * This is the opposite of traceeval_insert(). Instead of inserting
> + * an item into the traceeval historgram, it removes it.
> + *
> + * Returns 1 if it found and removed an item,
> + *         0 if it did not find an time matching @keys
> + *        -1 if there was an error.
> + */
> +int traceeval_remove(struct traceeval *teval,
> +		     const union traceeval_data *keys)
> +{
> +	struct hash_table *hist = teval->hist;
> +	struct entry *entry;
> +	int check;
> +
> +	entry = NULL;
> +	check = get_entry(teval, keys, &entry);
> +
> +	if (check < 1)
> +		return check;
> +
> +	hash_remove(hist, &entry->hash);
> +	return 1;
> +}
> +
>  /**
>   * traceeval_iterator_put - release a given iterator
>   * @iter: The iterartor to release
> -- 
> 2.40.1
>