On Wed, 3 Nov 2021 18:17:26 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> On Wed, 3 Nov 2021 13:03:19 -0400
> Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> > On Wed, 3 Nov 2021 17:44:08 +0200
> > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
> >
> > > +static int dyn_generic_parse(struct dyn_events_desc *desc, const char *group,
> > > + char *line, struct tracefs_dynevent **ret_dyn)
> > > +{
> > > + struct tracefs_dynevent *dyn;
> > > + char *format = NULL;
> > > + char *address;
> > > + char *system;
> > > + char *prefix;
> > > + char *event;
> > > + char *sav;
> > > +
> > > + if (strncmp(line, desc->prefix, strlen(desc->prefix)))
> > > + return -1;
> > > +
> > > + prefix = strtok_r(line, ":", &sav);
> > > + if (!prefix)
> > > + return -1;
> >
> > What if the user adds a name for the event?
> >
> > p:name system/event
> >
> > ?
>
> Actually, I'm curious to why we parse the prefix? We have it when it gets
> called. Why not use it?
OK, now looking at how this is used in this patch, I have a bit more
context. I replied before reading the rest of the patch, as this is
used to parse the actual file and not input.
This could definitely use some context. Especially function pointers in
structures. There should be some comment by the structure to explain
what the functions are for.
-- Steve
|