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
