Re: [PATCH 5/7] perf ui/browser: Add a browser for perf script

[Namhyung Kim <namhyung@kernel.org>]

On Mon,  3 Sep 2012 16:14:31 +0800, Feng Tang wrote:
> Create a script browser, so that user can check all the available
> scripts and run them inside the main perf report or annotation
> browsers, for all the perf samples or samples belong to one
> thread/symbol.
>
> The work flow is, users can use function key to list all the available
> scripts in system and chose one, which will be executed with
> popen("perf script -s xxx.xx",) and all the output lines are put into
> one ui browser, pressing 'q' or left arrow key will make it return
> to previous browser.

It could be used for TUI interface of perf script itself too (at least
for --list option) IMHO.


>
> Please be noted, most of the in-tree scripts with name xxxtop are
> dynamically run and not supported by this static browser. 

If so, wouldn't it be better adding a filter not to show those
unsupported scripts somehow?


> You can run the event_analyzing_sample.py for example.
>
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> ---
>  tools/perf/Makefile              |    5 +
>  tools/perf/ui/browsers/scripts.c |  159 ++++++++++++++++++++++++++++++++++++++
>  tools/perf/ui/browsers/scripts.h |    5 +
>  3 files changed, 169 insertions(+), 0 deletions(-)
>  create mode 100644 tools/perf/ui/browsers/scripts.c
>  create mode 100644 tools/perf/ui/browsers/scripts.h
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 722ddee..1fe23b9 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -569,6 +569,7 @@ else
>  		LIB_OBJS += $(OUTPUT)ui/browsers/annotate.o
>  		LIB_OBJS += $(OUTPUT)ui/browsers/hists.o
>  		LIB_OBJS += $(OUTPUT)ui/browsers/map.o
> +		LIB_OBJS += $(OUTPUT)ui/browsers/scripts.o
>  		LIB_OBJS += $(OUTPUT)ui/progress.o
>  		LIB_OBJS += $(OUTPUT)ui/util.o
>  		LIB_OBJS += $(OUTPUT)ui/tui/setup.o
> @@ -576,6 +577,7 @@ else
>  		LIB_OBJS += $(OUTPUT)ui/tui/helpline.o
>  		LIB_H += ui/browser.h
>  		LIB_H += ui/browsers/map.h
> +		LIB_H += ui/browsers/scripts.h
>  		LIB_H += ui/keysyms.h
>  		LIB_H += ui/libslang.h
>  		LIB_H += ui/progress.h
> @@ -878,6 +880,9 @@ $(OUTPUT)ui/browsers/hists.o: ui/browsers/hists.c $(OUTPUT)PERF-CFLAGS
>  $(OUTPUT)ui/browsers/map.o: ui/browsers/map.c $(OUTPUT)PERF-CFLAGS
>  	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<
>
> +$(OUTPUT)ui/browsers/scripts.o: ui/browsers/scripts.c $(OUTPUT)PERF-CFLAGS
> +	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<
> +
>  $(OUTPUT)util/rbtree.o: ../../lib/rbtree.c $(OUTPUT)PERF-CFLAGS
>  	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<
>
> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> new file mode 100644
> index 0000000..7183795
> --- /dev/null
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -0,0 +1,159 @@
> +#include <elf.h>
> +#include <newt.h>
> +#include <inttypes.h>
> +#include <sys/ttydefaults.h>
> +#include <string.h>
> +#include <linux/bitops.h>
> +#include "../../util/sort.h"
> +#include "../../util/util.h"
> +#include "../../util/debug.h"
> +#include "../../util/symbol.h"
> +#include "../browser.h"
> +#include "../helpline.h"
> +#include "../libslang.h"
> +#include "scripts.h"
> +
> +/* 2048 lines should be enough for a script output */
> +#define MAX_LINES		2048
> +
> +/* 160 bytes for one output line */
> +#define AVERAGE_LINE_LEN	160
> +
> +struct script_line {
> +	struct list_head node;
> +	char line[AVERAGE_LINE_LEN];
> +};
> +
> +struct perf_script_browser {
> +	struct ui_browser b;
> +	struct list_head entries;
> +	const char *script_name;
> +	int nr_lines;
> +};
> +
> +#define SCRIPT_NAMELEN	40
> +#define SCRIPT_MAX_NO	32
> +#define SCRIPT_FULLPATH_LEN 128

I'm not sure these magic numbers are enough.


> +
> +/* Return 0 on success */
> +static int list_scripts(char *script_name)
> +{
> +	char *buffer, *scripts[SCRIPT_MAX_NO], *scripts_path[SCRIPT_MAX_NO];
> +	int i, num, choice, ret = -1;
> +
> +	/* Preset the script name to SCRIPT_NAMELEN */
> +	buffer = zalloc(SCRIPT_MAX_NO * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN));
> +	if (!buffer)
> +		return ret;
> +
> +	for (i = 0; i < SCRIPT_MAX_NO; i++) {
> +		scripts[i] = buffer + i * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN);
> +		scripts_path[i] = scripts[i] + SCRIPT_NAMELEN;
> +	}
> +
> +	num = find_scripts(scripts, scripts_path);
> +	if (num) {

It should be:

	if (num > 0) {

since the find_scripts() can return -1.


> +		choice = ui__popup_menu(num, scripts);
> +		if (choice < num && choice >= 0) {
> +			strcpy(script_name, scripts_path[choice]);
> +			ret = 0;
> +		}
> +	}
> +
> +	free(buffer);
> +	return ret;
> +}
> +
> +static void script_browser__write(struct ui_browser *browser,
> +				   void *entry, int row)
> +{
> +	struct script_line *sline = list_entry(entry, struct script_line, node);
> +	bool current_entry = ui_browser__is_current_entry(browser, row);
> +
> +	ui_browser__set_color(browser, current_entry ? HE_COLORSET_SELECTED :
> +						       HE_COLORSET_NORMAL);
> +
> +	slsmg_write_nstring(sline->line, browser->width);
> +}
> +
> +static int script_browser__run(struct perf_script_browser *self)
> +{
> +	int key;
> +
> +	if (ui_browser__show(&self->b, self->script_name,
> +			     "Press <- or ESC to exit") < 0)
> +		return -1;
> +
> +	while (1) {
> +		key = ui_browser__run(&self->b, 0);
> +
> +		/* We can add some special key handling here if needed */
> +		break;
> +	}
> +
> +	ui_browser__hide(&self->b);
> +	return key;
> +}
> +
> +
> +int script_browse(const char *script_opt)
> +{
> +	char cmd[160], script_name[128];
> +	char *line = NULL;
> +	size_t len = 0;
> +	ssize_t retlen;
> +	int ret = -1, nr_entries = 0;
> +	FILE *fp;
> +	struct script_line *sline;
> +
> +	struct perf_script_browser script = {
> +		.b = {
> +			.refresh    = ui_browser__list_head_refresh,
> +			.seek	    = ui_browser__list_head_seek,
> +			.write	    = script_browser__write,
> +		},
> +		.script_name = script_name,
> +	};
> +
> +	INIT_LIST_HEAD(&script.entries);
> +
> +	/* Save each line of the output in one struct script_line object. */
> +	sline = malloc((sizeof(*sline)) * MAX_LINES);
> +	if (!sline)
> +		return -1;
> +
> +	memset(script_name, 0, 128);
> +	if (list_scripts(script_name))
> +		goto exit;
> +
> +	if (script_opt)
> +		sprintf(cmd, "perf script %s -s %s 2>&1", script_opt, script_name);
> +	else
> +		sprintf(cmd, "perf script -s %s 2>&1", script_name);
> +
> +	fp = popen(cmd, "r");
> +	if (!fp)
> +		goto exit;
> +
> +	while ((retlen = getline(&line, &len, fp)) != -1) {
> +		strcpy(sline[nr_entries].line, line);

What if the len is larger than sline[].line?


> +		list_add_tail(&sline[nr_entries].node, &script.entries);
> +
> +		if (script.b.width < retlen)
> +			script.b.width = retlen;
> +		nr_entries++;

I think you need this too:

		if (nr_entries >= MAX_LINES)
			break;
> +	}
> +
> +	script.nr_lines = nr_entries;
> +	script.b.nr_entries = nr_entries;
> +	script.b.entries = &script.entries;
> +
> +	ret = script_browser__run(&script);
> +
> +	if (line)
> +		free(line);

No pclose(fp) ?


> +
> +exit:
> +	free(sline);
> +	return ret;
> +}
> diff --git a/tools/perf/ui/browsers/scripts.h b/tools/perf/ui/browsers/scripts.h
> new file mode 100644
> index 0000000..011baa8
> --- /dev/null
> +++ b/tools/perf/ui/browsers/scripts.h
> @@ -0,0 +1,5 @@
> +#ifndef _PERF_UI_BROWSER_BROWSER_H_
> +#define _PERF_UI_BROWSER_BROWSER_H_ 1

I guess you want _PERF_UI_BROWSER_SCRIPT_H_ ?


> +
> +int script_browse(const char *script_opt);
> +#endif /* _PERF_UI_BROWSER_BROWSER_H_ */
> --
> 1.7.1