Messages in this thread | | | Date | Mon, 23 Sep 2019 16:12:53 +0530 | From | Srikar Dronamraju <> | Subject | Re: [for-next][PATCH 7/8] tracing/probe: Reject exactly same probe event |
| |
Hey Masami, Steven
> > +static bool trace_kprobe_has_same_kprobe(struct trace_kprobe *orig, > + struct trace_kprobe *comp) > +{ > + struct trace_probe_event *tpe = orig->tp.event; > + struct trace_probe *pos; > + int i; > + > + list_for_each_entry(pos, &tpe->probes, list) { > + orig = container_of(pos, struct trace_kprobe, tp); > + if (strcmp(trace_kprobe_symbol(orig), > + trace_kprobe_symbol(comp)) || > + trace_kprobe_offset(orig) != trace_kprobe_offset(comp)) > + continue; > + > + /* > + * trace_probe_compare_arg_type() ensured that nr_args and > + * each argument name and type are same. Let's compare comm. > + */ > + for (i = 0; i < orig->tp.nr_args; i++) { > + if (strcmp(orig->tp.args[i].comm, > + comp->tp.args[i].comm)) > + continue;
In a nested loop, *continue* is going to continue iterating through the inner loop. In which case, continue is doing nothing here. I thought we should have used a goto instead. No? To me, continue as a last statement of a for loop always looks weird.
> + } > + > + return true; > + }
I think we need something like this:
list_for_each_entry(pos, &tpe->probes, list) { orig = container_of(pos, struct trace_kprobe, tp); if (strcmp(trace_kprobe_symbol(orig), trace_kprobe_symbol(comp)) || trace_kprobe_offset(orig) != trace_kprobe_offset(comp)) continue;
/* * trace_probe_compare_arg_type() ensured that nr_args and * each argument name and type are same. Let's compare comm. */ for (i = 0; i < orig->tp.nr_args; i++) { if (strcmp(orig->tp.args[i].comm, comp->tp.args[i].comm)) goto outer_loop;
}
return true; outer_loop: }
> + > + return false; > +} > + >
......
> +static bool trace_uprobe_has_same_uprobe(struct trace_uprobe *orig, > + struct trace_uprobe *comp) > +{ > + struct trace_probe_event *tpe = orig->tp.event; > + struct trace_probe *pos; > + struct inode *comp_inode = d_real_inode(comp->path.dentry); > + int i; > + > + list_for_each_entry(pos, &tpe->probes, list) { > + orig = container_of(pos, struct trace_uprobe, tp); > + if (comp_inode != d_real_inode(orig->path.dentry) || > + comp->offset != orig->offset) > + continue; > + > + /* > + * trace_probe_compare_arg_type() ensured that nr_args and > + * each argument name and type are same. Let's compare comm. > + */ > + for (i = 0; i < orig->tp.nr_args; i++) { > + if (strcmp(orig->tp.args[i].comm, > + comp->tp.args[i].comm)) > + continue;
Same as above.
> + } > + > + return true; > + } > + > + return false; > +} > +
-- Thanks and Regards Srikar Dronamraju
| |