Re: [PATCH v6 07/22] perf script: Move find_scripts to browser/scripts.c

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Nov 08, 2024 at 10:17:54PM -0800, Ian Rogers wrote:
> The only use of find_scripts is in browser/scripts.c but the
> definition in builtin causes linking problems requiring a stub in
> python.c. Move the function to allow the stub to be removed.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  tools/perf/builtin-script.c      | 171 -----------------------------
>  tools/perf/builtin.h             |   6 --
>  tools/perf/ui/browsers/scripts.c | 177 ++++++++++++++++++++++++++++++-
>  tools/perf/util/python.c         |   6 --
>  4 files changed, 175 insertions(+), 185 deletions(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index e20d55b8a741..e9ec74056f71 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -3521,177 +3521,6 @@ static void free_dlarg(void)
>  	free(dlargv);
>  }
>  
> -/*
> - * Some scripts specify the required events in their "xxx-record" file,
> - * this function will check if the events in perf.data match those
> - * mentioned in the "xxx-record".
> - *
> - * Fixme: All existing "xxx-record" are all in good formats "-e event ",
> - * which is covered well now. And new parsing code should be added to
> - * cover the future complex formats like event groups etc.
> - */
> -static int check_ev_match(int dir_fd, const char *scriptname, struct perf_session *session)
> -{
> -	char line[BUFSIZ];
> -	FILE *fp;
> -
> -	{
> -		char filename[FILENAME_MAX + 5];
> -		int fd;
> -
> -		scnprintf(filename, sizeof(filename), "bin/%s-record", scriptname);
> -		fd = openat(dir_fd, filename, O_RDONLY);
> -		if (fd == -1)
> -			return -1;
> -		fp = fdopen(fd, "r");
> -		if (!fp)
> -			return -1;
> -	}
> -
> -	while (fgets(line, sizeof(line), fp)) {
> -		char *p = skip_spaces(line);
> -
> -		if (*p == '#')
> -			continue;
> -
> -		while (strlen(p)) {
> -			int match, len;
> -			struct evsel *pos;
> -			char evname[128];
> -
> -			p = strstr(p, "-e");
> -			if (!p)
> -				break;
> -
> -			p += 2;
> -			p = skip_spaces(p);
> -			len = strcspn(p, " \t");
> -			if (!len)
> -				break;
> -
> -			snprintf(evname, len + 1, "%s", p);
> -
> -			match = 0;
> -			evlist__for_each_entry(session->evlist, pos) {
> -				if (evsel__name_is(pos, evname)) {
> -					match = 1;
> -					break;
> -				}
> -			}
> -
> -			if (!match) {
> -				fclose(fp);
> -				return -1;
> -			}
> -		}
> -	}
> -
> -	fclose(fp);
> -	return 0;
> -}
> -
> -/*
> - * Return -1 if none is found, otherwise the actual scripts number.
> - *
> - * Currently the only user of this function is the script browser, which
> - * will list all statically runnable scripts, select one, execute it and
> - * show the output in a perf browser.
> - */
> -int find_scripts(char **scripts_array, char **scripts_path_array, int num,
> -		 int pathlen)
> -{
> -	struct dirent *script_dirent, *lang_dirent;
> -	int scripts_dir_fd, lang_dir_fd;
> -	DIR *scripts_dir, *lang_dir;
> -	struct perf_session *session;
> -	struct perf_data data = {
> -		.path = input_name,
> -		.mode = PERF_DATA_MODE_READ,
> -	};
> -	char *temp;
> -	int i = 0;
> -	const char *exec_path = get_argv_exec_path();
> -
> -	session = perf_session__new(&data, NULL);
> -	if (IS_ERR(session))
> -		return PTR_ERR(session);
> -
> -	{
> -		char scripts_path[PATH_MAX];
> -
> -		snprintf(scripts_path, sizeof(scripts_path), "%s/scripts", exec_path);
> -		scripts_dir_fd = open(scripts_path, O_DIRECTORY);
> -		pr_err("Failed to open directory '%s'", scripts_path);
> -		if (scripts_dir_fd == -1) {
> -			perf_session__delete(session);
> -			return -1;
> -		}
> -	}
> -	scripts_dir = fdopendir(scripts_dir_fd);
> -	if (!scripts_dir) {
> -		close(scripts_dir_fd);
> -		perf_session__delete(session);
> -		return -1;
> -	}
> -
> -	while ((lang_dirent = readdir(scripts_dir)) != NULL) {
> -		if (lang_dirent->d_type != DT_DIR &&
> -		    (lang_dirent->d_type == DT_UNKNOWN &&
> -		     !is_directory_at(scripts_dir_fd, lang_dirent->d_name)))
> -			continue;
> -		if (!strcmp(lang_dirent->d_name, ".") || !strcmp(lang_dirent->d_name, ".."))
> -			continue;
> -
> -#ifndef HAVE_LIBPERL_SUPPORT
> -		if (strstr(lang_dirent->d_name, "perl"))
> -			continue;
> -#endif
> -#ifndef HAVE_LIBPYTHON_SUPPORT
> -		if (strstr(lang_dirent->d_name, "python"))
> -			continue;
> -#endif
> -
> -		lang_dir_fd = openat(scripts_dir_fd, lang_dirent->d_name, O_DIRECTORY);
> -		if (lang_dir_fd == -1)
> -			continue;
> -		lang_dir = fdopendir(lang_dir_fd);
> -		if (!lang_dir) {
> -			close(lang_dir_fd);
> -			continue;
> -		}
> -		while ((script_dirent = readdir(lang_dir)) != NULL) {
> -			if (script_dirent->d_type == DT_DIR)
> -				continue;
> -			if (script_dirent->d_type == DT_UNKNOWN &&
> -			    is_directory_at(lang_dir_fd, script_dirent->d_name))
> -				continue;
> -			/* Skip those real time scripts: xxxtop.p[yl] */
> -			if (strstr(script_dirent->d_name, "top."))
> -				continue;
> -			if (i >= num)
> -				break;
> -			scnprintf(scripts_path_array[i], pathlen, "%s/scripts/%s/%s",
> -				exec_path,
> -				lang_dirent->d_name,
> -				script_dirent->d_name);
> -			temp = strchr(script_dirent->d_name, '.');
> -			snprintf(scripts_array[i],
> -				(temp - script_dirent->d_name) + 1,
> -				"%s", script_dirent->d_name);
> -
> -			if (check_ev_match(lang_dir_fd, scripts_array[i], session))
> -				continue;
> -
> -			i++;
> -		}
> -		closedir(lang_dir);
> -	}
> -
> -	closedir(scripts_dir);
> -	perf_session__delete(session);
> -	return i;
> -}
> -
>  static char *get_script_path(const char *script_root, const char *suffix)
>  {
>  	struct dirent *script_dirent, *lang_dirent;
> diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
> index 94f4b3769bf7..a07e93c53848 100644
> --- a/tools/perf/builtin.h
> +++ b/tools/perf/builtin.h
> @@ -2,10 +2,6 @@
>  #ifndef BUILTIN_H
>  #define BUILTIN_H
>  
> -#include <stddef.h>
> -#include <linux/compiler.h>
> -#include <tools/config.h>
> -
>  struct feature_status {
>  	const char *name;
>  	const char *macro;
> @@ -56,6 +52,4 @@ int cmd_ftrace(int argc, const char **argv);
>  int cmd_daemon(int argc, const char **argv);
>  int cmd_kwork(int argc, const char **argv);
>  
> -int find_scripts(char **scripts_array, char **scripts_path_array, int num,
> -		 int pathlen);
>  #endif
> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> index e437d7889de6..2d04ece833aa 100644
> --- a/tools/perf/ui/browsers/scripts.c
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -1,16 +1,18 @@
>  // SPDX-License-Identifier: GPL-2.0
> -#include "../../builtin.h"
> -#include "../../perf.h"
>  #include "../../util/util.h" // perf_exe()
>  #include "../util.h"
> +#include "../../util/evlist.h"
>  #include "../../util/hist.h"
>  #include "../../util/debug.h"
> +#include "../../util/session.h"
>  #include "../../util/symbol.h"
>  #include "../browser.h"
>  #include "../libslang.h"
>  #include "config.h"
> +#include <linux/err.h>
>  #include <linux/string.h>
>  #include <linux/zalloc.h>
> +#include <subcmd/exec-cmd.h>
>  #include <stdlib.h>
>  
>  #define SCRIPT_NAMELEN	128
> @@ -77,6 +79,177 @@ static int scripts_config(const char *var, const char *value, void *data)
>  	return 0;
>  }
>  
> +/*
> + * Some scripts specify the required events in their "xxx-record" file,
> + * this function will check if the events in perf.data match those
> + * mentioned in the "xxx-record".
> + *
> + * Fixme: All existing "xxx-record" are all in good formats "-e event ",
> + * which is covered well now. And new parsing code should be added to
> + * cover the future complex formats like event groups etc.
> + */
> +static int check_ev_match(int dir_fd, const char *scriptname, struct perf_session *session)
> +{
> +	char line[BUFSIZ];
> +	FILE *fp;
> +
> +	{
> +		char filename[FILENAME_MAX + 5];
> +		int fd;
> +
> +		scnprintf(filename, sizeof(filename), "bin/%s-record", scriptname);
> +		fd = openat(dir_fd, filename, O_RDONLY);
> +		if (fd == -1)
> +			return -1;
> +		fp = fdopen(fd, "r");
> +		if (!fp)
> +			return -1;
> +	}
> +
> +	while (fgets(line, sizeof(line), fp)) {
> +		char *p = skip_spaces(line);
> +
> +		if (*p == '#')
> +			continue;
> +
> +		while (strlen(p)) {
> +			int match, len;
> +			struct evsel *pos;
> +			char evname[128];
> +
> +			p = strstr(p, "-e");
> +			if (!p)
> +				break;
> +
> +			p += 2;
> +			p = skip_spaces(p);
> +			len = strcspn(p, " \t");
> +			if (!len)
> +				break;
> +
> +			snprintf(evname, len + 1, "%s", p);
> +
> +			match = 0;
> +			evlist__for_each_entry(session->evlist, pos) {
> +				if (evsel__name_is(pos, evname)) {
> +					match = 1;
> +					break;
> +				}
> +			}
> +
> +			if (!match) {
> +				fclose(fp);
> +				return -1;
> +			}
> +		}
> +	}
> +
> +	fclose(fp);
> +	return 0;
> +}
> +
> +/*
> + * Return -1 if none is found, otherwise the actual scripts number.
> + *
> + * Currently the only user of this function is the script browser, which
> + * will list all statically runnable scripts, select one, execute it and
> + * show the output in a perf browser.
> + */
> +static int find_scripts(char **scripts_array, char **scripts_path_array, int num,
> +		 int pathlen)
> +{
> +	struct dirent *script_dirent, *lang_dirent;
> +	int scripts_dir_fd, lang_dir_fd;
> +	DIR *scripts_dir, *lang_dir;
> +	struct perf_session *session;
> +	struct perf_data data = {
> +		.path = input_name,
> +		.mode = PERF_DATA_MODE_READ,
> +	};
> +	char *temp;
> +	int i = 0;
> +	const char *exec_path = get_argv_exec_path();
> +
> +	session = perf_session__new(&data, NULL);
> +	if (IS_ERR(session))
> +		return PTR_ERR(session);
> +
> +	{
> +		char scripts_path[PATH_MAX];
> +
> +		snprintf(scripts_path, sizeof(scripts_path), "%s/scripts", exec_path);
> +		scripts_dir_fd = open(scripts_path, O_DIRECTORY);
> +		pr_err("Failed to open directory '%s'", scripts_path);
> +		if (scripts_dir_fd == -1) {
> +			perf_session__delete(session);
> +			return -1;
> +		}
> +	}
> +	scripts_dir = fdopendir(scripts_dir_fd);
> +	if (!scripts_dir) {
> +		close(scripts_dir_fd);
> +		perf_session__delete(session);
> +		return -1;
> +	}
> +
> +	while ((lang_dirent = readdir(scripts_dir)) != NULL) {
> +		if (lang_dirent->d_type != DT_DIR &&
> +		    (lang_dirent->d_type == DT_UNKNOWN &&
> +		     !is_directory_at(scripts_dir_fd, lang_dirent->d_name)))
> +			continue;
> +		if (!strcmp(lang_dirent->d_name, ".") || !strcmp(lang_dirent->d_name, ".."))
> +			continue;
> +
> +#ifndef HAVE_LIBPERL_SUPPORT
> +		if (strstr(lang_dirent->d_name, "perl"))
> +			continue;
> +#endif
> +#ifndef HAVE_LIBPYTHON_SUPPORT
> +		if (strstr(lang_dirent->d_name, "python"))
> +			continue;
> +#endif
> +
> +		lang_dir_fd = openat(scripts_dir_fd, lang_dirent->d_name, O_DIRECTORY);
> +		if (lang_dir_fd == -1)
> +			continue;
> +		lang_dir = fdopendir(lang_dir_fd);
> +		if (!lang_dir) {
> +			close(lang_dir_fd);
> +			continue;
> +		}
> +		while ((script_dirent = readdir(lang_dir)) != NULL) {
> +			if (script_dirent->d_type == DT_DIR)
> +				continue;
> +			if (script_dirent->d_type == DT_UNKNOWN &&
> +			    is_directory_at(lang_dir_fd, script_dirent->d_name))
> +				continue;
> +			/* Skip those real time scripts: xxxtop.p[yl] */
> +			if (strstr(script_dirent->d_name, "top."))
> +				continue;
> +			if (i >= num)
> +				break;
> +			scnprintf(scripts_path_array[i], pathlen, "%s/scripts/%s/%s",
> +				exec_path,
> +				lang_dirent->d_name,
> +				script_dirent->d_name);
> +			temp = strchr(script_dirent->d_name, '.');
> +			snprintf(scripts_array[i],
> +				(temp - script_dirent->d_name) + 1,
> +				"%s", script_dirent->d_name);
> +
> +			if (check_ev_match(lang_dir_fd, scripts_array[i], session))
> +				continue;
> +
> +			i++;
> +		}
> +		closedir(lang_dir);
> +	}
> +
> +	closedir(scripts_dir);
> +	perf_session__delete(session);
> +	return i;
> +}
> +
>  /*
>   * When success, will copy the full path of the selected script
>   * into  the buffer pointed by script_name, and return 0.
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 94902652e371..eb15f3b6c4f5 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -1307,12 +1307,6 @@ PyMODINIT_FUNC PyInit_perf(void)
>  /* The following are stubs to avoid dragging in builtin-* objects. */
>  /* TODO: move the code out of the builtin-* file into util. */
>  
> -int find_scripts(char **scripts_array  __maybe_unused, char **scripts_path_array  __maybe_unused,
> -		int num  __maybe_unused, int pathlen __maybe_unused)
> -{
> -	return -1;
> -}
> -
>  void perf_stat__set_no_csv_summary(int set __maybe_unused)
>  {
>  }
> -- 
> 2.47.0.277.g8800431eea-goog
>