On Thu, Sep 22, 2022 at 1:00 PM Sunil Kumar Kori <sk...@marvell.com> wrote:
> > @@ -210,15 +210,18 @@ rte_trace_regexp(const char *regex, bool enable)
> > return -EINVAL;
> >
> > STAILQ_FOREACH(tp, &tp_list, next) {
> > - if (regexec(&r, tp->name, 0, NULL, 0) == 0) {
> > - if (enable)
> > - rc = rte_trace_point_enable(tp->handle);
> > - else
> > - rc = rte_trace_point_disable(tp->handle);
> > - found = 1;
> > + if (regexec(&r, tp->name, 0, NULL, 0) != 0)
> > + continue;
> > +
> > + if (enable)
> > + rc = rte_trace_point_enable(tp->handle);
> > + else
> > + rc = rte_trace_point_disable(tp->handle);
> > + if (rc < 0) {
> > + found = 0;
> > + break;
> > }
> > - if (rc < 0)
> > - return rc;
> > + found = 1;
> > }
> > regfree(&r);
> >
> > --
>
> I understand the problem addressed by this fix but may be following changes
> will be sufficient to fix it.
> Please highlight, If I am missing. Just trying to reduce the line of changes.
>
> @@ -220,8 +220,10 @@ rte_trace_regexp(const char *regex, bool enable)
> rc = rte_trace_point_disable(tp->handle);
> found = 1;
> }
> - if (rc < 0)
> - return rc;
> + if (rc < 0) {
> + found = 0;
> + break;
> + }
> }
rc is compared against 0 for all non-matching tracepoints.
This is unnecessary and fragile.
I can split this change in two: one for the fix, and one other where
the loop is updated with a continue and an inverted matching condition
like above.
If going like this, I would update rte_trace_pattern() too, in the second patch.
WDYT?
--
David Marchand
