On Mon, 10 Mar 2014 20:01:34 +0000 (UTC) Mathieu Desnoyers <mathieu.desnoy...@efficios.com> wrote: > > mutex_lock(&tracepoints_mutex); > > old = tracepoint_add_probe(name, probe, data); > > @@ -388,9 +393,13 @@ int tracepoint_probe_register(const char *name, void > > *probe, void *data) > > return PTR_ERR(old); > > } > > tracepoint_update_probes(); /* may update entry */ > > + entry = get_tracepoint(name); > > + /* Make sure the entry was enabled */ > > + if (!entry || !entry->enabled) > > + ret = -ENODEV; > > Hi Steven, > > Returning -ENODEV when the probe is still registered might come as a > surprise to the caller. For instance, a caller may dynamically allocate > name, probe, and/or data, it may want to free them when > tracepoint_probe_register returns an error. But this "-ENODEV" return value > is not really an error, and the parameters passed are still used.
It's an error when you wanted to enable a probe and the probe doesn't exist. There are no in tree users of this call that expect it to work when the probe does not exist. > > If we go down this route, we might want at the very least to add documentation > of tracepoint_probe_register() return values and their meaning > in a comment on top of this function (perhaps also in the header). But > even if we do so, this weird return value semantic with respect to use of the > received parameters will likely cause memory corruption at some point. > > Thoughts ? Send a patch to document the return values. Your module can expect this return value when it doesn't expect the probe to exist. Again, it's really strange when you go to enable a probe, and there is no probe to enable. Note, I was nice. I removed the logic to unregister the probe in this case. Anyway, this should even help you. Before there was no way to enable a probe and know if it was enabled or not. That is, if it didn't exist, there was no feedback letting you know that. If you expect to enable a probe that doesn't exist, then you can expect this return value. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
