From: Ross Zwisler <zwisler@google.com>
To: Stevie Alvarez <stevie.6strings@gmail.com>
Cc: linux-trace-devel@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH v2 4/5] histograms: Add traceeval compare
Date: Mon, 7 Aug 2023 17:40:13 -0600 [thread overview]
Message-ID: <20230807234013.GA99888@google.com> (raw)
In-Reply-To: <20230803225413.40697-5-stevie.6strings@gmail.com>
On Thu, Aug 03, 2023 at 06:54:02PM -0400, Stevie Alvarez wrote:
> From: "Stevie Alvarez (Google)" <stevie.6strings@gmail.com>
>
> 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@gmail.com>
> ---
> 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)
next prev parent reply other threads:[~2023-08-07 23:40 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-03 22:53 [PATCH v2 0/5] histograms: bug fixes and convention compliance Stevie Alvarez
2023-08-03 22:53 ` [PATCH v2 1/5] histograms: Initial histograms interface Stevie Alvarez
2023-08-04 13:50 ` Steven Rostedt
2023-08-04 17:41 ` Stevie Alvarez
2023-08-04 17:57 ` Steven Rostedt
2023-08-04 18:25 ` Stevie Alvarez
2023-08-03 22:54 ` [PATCH v2 2/5] histograms: Add traceeval initialize Stevie Alvarez
2023-08-04 14:40 ` Steven Rostedt
2023-08-04 18:23 ` Stevie Alvarez
2023-08-04 19:20 ` Steven Rostedt
2023-08-03 22:54 ` [PATCH v2 3/5] histograms: Add traceeval release Stevie Alvarez
2023-08-04 14:55 ` Steven Rostedt
2023-08-03 22:54 ` [PATCH v2 4/5] histograms: Add traceeval compare Stevie Alvarez
2023-08-04 15:24 ` Steven Rostedt
2023-08-07 23:40 ` Ross Zwisler [this message]
2023-08-03 22:54 ` [PATCH v2 5/5] histograms: Initial unit tests Stevie Alvarez
2023-08-04 12:37 ` [PATCH v2 0/5] histograms: bug fixes and convention compliance Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230807234013.GA99888@google.com \
--to=zwisler@google.com \
--cc=linux-trace-devel@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=stevie.6strings@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).