On Fri, Jul 28, 2023 at 03:04:39PM -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>
> ---
> src/histograms.c | 178 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 176 insertions(+), 2 deletions(-)
>
> diff --git a/src/histograms.c b/src/histograms.c
> index f46a0e0..5f1c7ef 100644
> --- a/src/histograms.c
> +++ b/src/histograms.c
> @@ -20,6 +20,19 @@
> #define for_each_key(i, keys) \
> for (i = 0; (keys)[(i)].type != TRACEEVAL_TYPE_NONE; (i)++)
>
> +/**
> + * Compare two integers of variable length.
> + *
> + * Return 0 if @a and @b are the same, 1 if @a is greater than @b, and -1
> + * if @b is greater than @a.
> + */
> +#define compare_numbers_return(a, b) \
> +do { \
> + if ((a) < (b)) \
> + return -1; \
> + return (a) != (b); \
> +} while (0) \
> +
> /** A key-value pair */
> struct entry {
> union traceeval_data *keys;
> @@ -56,10 +69,171 @@ static void print_err(const char *fmt, ...)
> fprintf(stderr, "\n");
> }
>
> -// TODO
> +/**
> + * Return 0 if @orig and @copy are the same, 1 otherwise.
> + */
> +static int compare_traceeval_type(struct traceeval_type *orig,
> + struct traceeval_type *copy)
> +{
> + int o_name_null;
> + int c_name_null;
> +
> + // same memory/null
> + if (orig == copy)
> + return 0;
> +
> + size_t i = 0;
Best practice is to put all the variable definitions at the top of the
function.
> + do {
> + if (orig[i].type != copy[i].type)
> + return 1;
> + if (orig[i].type == TRACEEVAL_TYPE_NONE)
> + return 0;
> + 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
> + o_name_null = !orig[i].name;
> + c_name_null = !copy[i].name;
> + if (o_name_null != c_name_null)
> + return 1;
> + if (o_name_null)
> + continue;
> + if (strcmp(orig[i].name, copy[i].name) != 0)
> + return 1;
> + } while (orig[i++].type != TRACEEVAL_TYPE_NONE);
> +
> + return 0;
> +}
<snip>
> +/**
> + * Return 0 if struct hist_table of @orig and @copy are the same, 1 if not,
> + * and -1 on error.
> + */
> +static int compare_hist(struct traceeval *orig, struct traceeval *copy)
> +{
> + struct hist_table *o_hist = orig->hist;
> + struct hist_table *c_hist = copy->hist;
> + int cnt = !(o_hist->nr_entries == c_hist->nr_entries);
> + if (cnt)
> + return 1;
> +
> + for (size_t i = 0; i < o_hist->nr_entries; i++) {
> + // cmp each entry
> + compare_entries(&o_hist->map[i], &c_hist->map[i], orig);
Need to check the return value of 'compare_entries()' and return it if it's
nonzero.
> + }
> + return 0;
> +}
> +
> +/**
> + * Return 0 if @orig and @copy are the same, 1 if not, -1 if error.
> + */
> int traceeval_compare(struct traceeval *orig, struct traceeval *copy)
> {
> - return -1;
> + if ((!orig) || (!copy))
No need for parens, this can just be:
if (!orig || !copy)
return -1;
> + return -1;
> +
> + int keys = compare_traceeval_type(orig->def_keys, copy->def_keys);
> + int vals = compare_traceeval_type(orig->def_vals, copy->def_vals);
> + int hists = compare_hist(orig, copy);
> +
> + return (keys || vals || hists);
> }
>
> /**
> --
> 2.41.0
>
|