On Thu, Aug 03, 2023 at 06:54:02PM -0400, Stevie Alvarez wrote:
> From: "Stevie Alvarez (Google)" <stevie.6strings@xxxxxxxxx>
>
> traceeval_compare() compares two struct traceeval instances for
> equality. This suite of comparitors was made for testing purposes.
>
> Signed-off-by: Stevie Alvarez (Google) <stevie.6strings@xxxxxxxxx>
> ---
> include/traceeval-test.h | 16 +++
> src/histograms.c | 221 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 234 insertions(+), 3 deletions(-)
> create mode 100644 include/traceeval-test.h
>
> diff --git a/include/traceeval-test.h b/include/traceeval-test.h
> new file mode 100644
> index 0000000..bb8092a
> --- /dev/null
> +++ b/include/traceeval-test.h
<>
> @@ -158,12 +172,13 @@ fail_type_name:
> * The @keys and @vals passed in are copied for internal use.
> *
> * For any member of @keys or @vals that isn't of type TRACEEVAL_TYPE_NONE,
> - * the name field must be a null-terminated string. For members of type
> - * TRACEEVAL_TYPE_NONE, the name is ignored.
> + * the name field must be a null-terminated string. Members of type
> + * TRACEEVAL_TYPE_NONE are used to terminate the array, therefore their other
> + * fields are ignored.
> *
> * @vals can be NULL or start with its type field as TRACEEVAL_TYPE_NONE to
> * define the values of the histogram to be empty.
> - * @keys must be populated with at least one element that is not
> + * @keys must be populated with at least one element that is not of type
These two comment updates seem accidental, and can be just folded into the
initial patch that creates them.
> * TRACEEVAL_TYPE_NONE.
> *
> * Returns the descriptor on success, or NULL on error.
> @@ -305,3 +320,203 @@ void traceeval_release(struct traceeval *teval)
> teval->val_types = NULL;
> free(teval);
> }
> +
> +/*
> + * Compare traceeval_type instances for equality.
> + *
> + * Return 0 if @orig and @copy are the same, 1 otherwise.
> + */
> +static int compare_traceeval_type(struct traceeval_type *orig,
> + struct traceeval_type *copy, size_t orig_size, size_t copy_size)
> +{
> + size_t i;
> +
> + /* same memory/NULL */
> + if (orig == copy)
> + return 0;
> + if (!!orig != !!copy)
> + return 1;
> +
> + if (orig_size != copy_size)
> + return 1;
> +
> + for (i = 0; i < orig_size; i++) {
> + if (orig[i].type != copy[i].type)
> + return 1;
> + if (orig[i].flags != copy[i].flags)
> + return 1;
> + if (orig[i].id != copy[i].id)
> + return 1;
> + if (orig[i].dyn_release != copy[i].dyn_release)
> + return 1;
> + if (orig[i].dyn_cmp != copy[i].dyn_cmp)
> + return 1;
> +
> + // make sure both names are same type
> + if (!!orig[i].name != !!copy[i].name)
> + return 1;
> + if (!orig[i].name)
> + continue;
> + if (strcmp(orig[i].name, copy[i].name) != 0)
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Compare traceeval_data instances.
> + *
> + * Return 0 if @orig and @copy are the same, 1 if @orig is greater than @copy,
> + * -1 for the other way around, and -2 on error.
> + */
> +static int compare_traceeval_data(union traceeval_data *orig,
For both this and compare_traceeval_data_set(), both 'orig' and 'copy' can be
const. Same with 'type', since compare functions shouldn't be modifying any
data. This applies to most or all of the pointer params to your compare
functions.
> + const union traceeval_data *copy, struct traceeval_type *type)
> +{
> + int i;
> +
> + if (orig == copy)
> + return 0;
> +
> + if (!orig)
> + return -1;
> +
> + if (!copy)
> + return 1;
> +
> + switch (type->type) {
> + case TRACEEVAL_TYPE_STRING:
> + i = strcmp(orig->string, copy->string);
> + compare_numbers_return(i, 0);
> +
> + case TRACEEVAL_TYPE_NUMBER:
> + compare_numbers_return(orig->number, copy->number);
> +
> + case TRACEEVAL_TYPE_NUMBER_64:
> + compare_numbers_return(orig->number_64, copy->number_64);
> +
> + case TRACEEVAL_TYPE_NUMBER_32:
> + compare_numbers_return(orig->number_32, copy->number_32);
> +
> + case TRACEEVAL_TYPE_NUMBER_16:
> + compare_numbers_return(orig->number_16, copy->number_16);
> +
> + case TRACEEVAL_TYPE_NUMBER_8:
> + 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);
> + return 0;
> +
> + default:
> + print_err("%d is an invalid enum traceeval_data_type member",
> + type->type);
> + return -2;
> + }
> +}
> +
> +/*
> + * Compare arrays of union traceeval_data's with respect to @def.
> + *
> + * Return 0 if @orig and @copy are the same, 1 if not, and -1 on error.
> + */
> +static int compare_traceeval_data_set(union traceeval_data *orig,
> + const union traceeval_data *copy, struct traceeval_type *defs,
> + size_t size)
|