On Wed, Aug 26, 2020 at 4:34 AM Jiri Olsa <jo...@redhat.com> wrote: > > On Tue, Aug 25, 2020 at 09:29:09PM -0700, Ian Rogers wrote: > > This patch resolves some undefined behavior where variables in > > expr_id_data were accessed (for debugging) without being defined. To > > better enforce the tagged union behavior, the struct is moved into > > expr.c and accessors provided. Tag values (kinds) are explicitly > > identified. > > > > Signed-off-by: Ian Rogers <irog...@google.com> > > --- > > tools/perf/util/expr.c | 64 ++++++++++++++++++++++++++++++----- > > tools/perf/util/expr.h | 17 +++------- > > tools/perf/util/expr.y | 2 +- > > tools/perf/util/metricgroup.c | 4 +-- > > 4 files changed, 62 insertions(+), 25 deletions(-) > > > > diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c > > index 53482ef53c41..1ca0992db86b 100644 > > --- a/tools/perf/util/expr.c > > +++ b/tools/perf/util/expr.c > > @@ -17,6 +17,25 @@ > > extern int expr_debug; > > #endif > > > > +struct expr_id_data { > > + union { > > + double val; > > + struct { > > + double val; > > + const char *metric_name; > > + const char *metric_expr; > > + } ref; > > + struct expr_id *parent; > > + }; > > + > > + enum { > > + EXPR_ID_DATA__VALUE, > > + EXPR_ID_DATA__REF, > > + EXPR_ID_DATA__REF_VALUE, > > + EXPR_ID_DATA__PARENT, > > + } kind; > > I like that, it's more clear than current state ;-) > > could you still put a small comment for each enum above, > as a hint what it's used for?
Thanks, I had a go at this in v2: https://lore.kernel.org/lkml/20200826153055.2067780-1-irog...@google.com/T/#u Ian > thanks, > jirka >