On Wed, Mar 06, 2024 at 03:53:57PM -0800, Dan Williams wrote:
> alison.schofield@ wrote:
> > From: Alison Schofield <[email protected]>
> >
> > Add helpers to extract the value of an event record field given the
> > field name. This is useful when the user knows the name and format
> > of the field and simply needs to get it.
> >
> > Since this is in preparation for adding a cxl_poison private parser
> > for 'cxl list --media-errors' support, add those specific required
> > types: u8, u32, u64, char*
> >
> > Signed-off-by: Alison Schofield <[email protected]>
> > Reviewed-by: Dave Jiang <[email protected]>
> > ---
> > cxl/event_trace.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++
> > cxl/event_trace.h | 10 ++++++-
> > 2 files changed, 84 insertions(+), 1 deletion(-)
> >
> > diff --git a/cxl/event_trace.c b/cxl/event_trace.c
> > index bdad0c19dbd4..6cc9444f3204 100644
> > --- a/cxl/event_trace.c
> > +++ b/cxl/event_trace.c
> > @@ -15,6 +15,81 @@
> > #define _GNU_SOURCE
> > #include <string.h>
> >
> > +static struct tep_format_field *__find_field(struct tep_event *event,
> > + const char *name)
> > +{
> > + struct tep_format_field **fields;
> > +
> > + fields = tep_event_fields(event);
> > + if (!fields)
> > + return NULL;
> > +
> > + for (int i = 0; fields[i]; i++) {
> > + struct tep_format_field *f = fields[i];
> > +
> > + if (strcmp(f->name, name) != 0)
> > + continue;
> > +
> > + return f;
> > + }
> > + return NULL;
> > +}
>
> Is this open-coded tep_find_field()?
Yes it is and now it is gone.
>
> > +
> > +u64 cxl_get_field_u64(struct tep_event *event, struct tep_record *record,
> > + const char *name)
> > +{
> > + struct tep_format_field *f;
> > + unsigned char *val;
> > + int len;
> > +
> > + f = __find_field(event, name);
> > + if (!f)
> > + return ULLONG_MAX;
> > +
> > + val = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
> > + if (!val)
> > + return ULLONG_MAX;
> > +
> > + return *(u64 *)val;
> > +}
>
> Is this just open-coded tep_get_any_field_val()?
It's a bit more. It returns ULLONG_MAX and casts to the u64 which
makes the call site cleaner.
I did change it to use tep_get_field_val(). Please look at next rev.
>
> > +
> > +char *cxl_get_field_string(struct tep_event *event, struct tep_record
> > *record,
> > + const char *name)
>
> Return a 'const char *'?
>
> > +{
> > + struct tep_format_field *f;
> > + int len;
> > +
> > + f = __find_field(event, name);
> > + if (!f)
> > + return NULL;
> > +
> > + return tep_get_field_raw(NULL, event, f->name, record, &len, 0);
>
> Is this guaranteed to be a string? ...and guaranteed to be NULL
> terminated?
>
It's gone. Using tep_get_field_raw() directly for str.
Thanks for reviewing!
