Re: [PATCH 1/2] [BUGFIX] perf probe: Fix --list option to show events only with uprobe events

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Aug 13, 2014 at 11:21:34PM +0900, Masami Hiramatsu escreveu:
> (2014/08/13 14:22), Namhyung Kim wrote:
> > On Wed, 13 Aug 2014 00:50:55 +0000, Masami Hiramatsu wrote:
> >> +	if (kp_fd < 0 && up_fd < 0) {
> >> +		/* Both kprobes and uprobes are disabled, warn it. */
> >> +		if (kp_fd == -ENOTSUP && up_fd == -ENOTSUP)
> >> +			pr_warning("Debugfs is not mounted.\n");
> >> +		else if (kp_fd == -ENOENT && up_fd == -ENOENT)
> >> +			pr_warning("Please rebuild kernel with "
> >> +				   "CONFIG_KPROBE_EVENTS or/and "
> >> +				   "CONFIG_UPROBE_EVENTS.\n");
> >> +		else
> >> +			pr_warning("Failed to open kprobe events: %s.\n" \
> >> +				   "Failed to open uprobe events: %s.\n",
> >> +				   strerror(-kp_fd), strerror(-up_fd));
> > 
> > It seems the second strerror() might overwrite the message of the
> > first.  You'd better using strerror_r() IMHO.

Well spotted!
 
> Oops, right, it must use the same buffer...
> But instead of using strerror_r, we can call pr_warning twice. Or should we
> better replace all strerror to strerror_r in perf? (it should be another series)

Well, don't introduce new strerror() uses, we have threads in perf
already and if both try to use strerror() for different reasons, say the
UI to print something to the user and some logging/debugging thread do
it to the disk, we may race.

So, in this case, please use strerror_r() and if you feel like
contributing the changes to any other place where strerror() is still
used, you are welcome to do so at a later patch :)

- Arnaldo