Re: [PATCH v2 09/11] libtraceeval: Make traceeval_remove() check size of keys array

[Ross Zwisler <zwisler@google.com>]

On Wed, Sep 27, 2023 at 08:33:12AM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> Make traceeval_remove() into a macro to pass in the size of the keys
> array, and rename the function to traceeval_remove_size() that now takes
> the size of the keys array.
> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  include/traceeval-hist.h | 7 +++++--
>  src/histograms.c         | 7 +++++--
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
> index 7f48bb92cc96..804a0aaa631d 100644
> --- a/include/traceeval-hist.h
> +++ b/include/traceeval-hist.h
> @@ -196,8 +196,11 @@ int traceeval_insert_size(struct traceeval *teval,
>  	traceeval_insert_size(teval, keys, TRACEEVAL_ARRAY_SIZE(keys), \
>  			      vals, TRACEEVAL_ARRAY_SIZE(vals))
>  
> -int traceeval_remove(struct traceeval *teval,
> -		     const struct traceeval_data *keys);
> +int traceeval_remove_size(struct traceeval *teval,
> +			  const struct traceeval_data *keys, size_t nr_keys);
> +
> +#define traceeval_remove(teval, keys)					\
> +	traceeval_remove_size(teval, keys, TRACEEVAL_ARRAY_SIZE(keys))
>  
>  int traceeval_query_size(struct traceeval *teval, const struct traceeval_data *keys,
>  			 size_t nr_keys, const struct traceeval_data **results);
> diff --git a/src/histograms.c b/src/histograms.c
> index ab8a560fe14d..28cf0d4ed225 100644
> --- a/src/histograms.c
> +++ b/src/histograms.c
> @@ -977,13 +977,16 @@ int traceeval_insert_size(struct traceeval *teval,

I think we need to update the comments above this function to account for the
new name and the additional 'nr_keys' param, but other than that you can add:

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

>   *         0 if it did not find an time matching @keys
>   *        -1 if there was an error.
>   */
> -int traceeval_remove(struct traceeval *teval,
> -		     const struct traceeval_data *keys)
> +int traceeval_remove_size(struct traceeval *teval,
> +			  const struct traceeval_data *keys, size_t nr_keys)
>  {
>  	struct hash_table *hist = teval->hist;
>  	struct entry *entry;
>  	int check;
>  
> +	if (teval->nr_key_types != nr_keys)
> +		return -1;
> +
>  	entry = NULL;
>  	check = get_entry(teval, keys, &entry);
>  
> -- 
> 2.40.1
>