On Thu, Aug 17, 2023 at 06:24:18PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
>
> Having a straight union for passing in the data that must match the types
> is dangerous and prone for buggy code. If some data doesn't match its
> type, there's nothing to catch it.
>
> Instead of having a union traceeval_data of each type, have it be a
> structure with a "type" field follow by a union, where the type defines
> what kind of data it is.
>
> Add helper macros to make it easy to define the data when using it.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
> include/traceeval-hist.h | 88 ++++++++++++++++---------
> samples/task-eval.c | 137 ++++++++++++++++-----------------------
> src/eval-local.h | 4 +-
> src/histograms.c | 68 +++++++++----------
> 4 files changed, 150 insertions(+), 147 deletions(-)
>
<>
> @@ -802,13 +779,9 @@ static void display_process_stats(struct traceeval *teval,
> {
> struct traceeval_stat *stat;
> unsigned long long delta;
> - union traceeval_data keys[] = {
> - {
> - .cstring = comm,
> - },
> - {
> - .number = RUNNING,
> - }
> + struct traceeval_data keys[] = {
> + DEFINE_TRACEEVAL_CSTRING( comm ),
> + DEFINE_TRACEEVAL_NUMBER( RUNNING ),
> };
>
> for (int i = 0; i < OTHER; i++) {
Just after this in display_process_stats we have:
> for (int i = 0; i < OTHER; i++) {
> keys[1].number = i;
Which I think we want to set with the new macro TRACEEVAL_SET_NUMBER().
TRACEEVAL_SET_NUMBER(keys[1], i)
Other than that you can add:
Reviewed-by: Ross Zwisler <zwisler@xxxxxxxxxx>
|