Re: [PATCH v5 08/21] perf script: Move script_spec code to trace-event-scripting.c

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

On Wed, Oct 30, 2024 at 06:42:39PM -0700, Ian Rogers wrote:
> The script_spec code is referenced in util/trace-event-scripting but
> the list was in builtin-script, accessed via a function that required
> a stub function in python.c. Move all the logic to
> trace-event-scripting, with lookup and foreach functions exposed for
> builtin-script's benefit.

Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/builtin-script.c             | 67 +---------------------
>  tools/perf/util/python.c                |  5 --
>  tools/perf/util/trace-event-scripting.c | 75 +++++++++++++++++++++++++
>  tools/perf/util/trace-event.h           |  3 +-
>  4 files changed, 80 insertions(+), 70 deletions(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 62e851fdf5ca..11c0ee8c1afc 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -2956,79 +2956,18 @@ static int __cmd_script(struct perf_script *script)
>  	return ret;
>  }
>  
> -struct script_spec {
> -	struct list_head	node;
> -	struct scripting_ops	*ops;
> -	char			spec[];
> -};
> -
> -static LIST_HEAD(script_specs);
> -
> -static struct script_spec *script_spec__new(const char *spec,
> -					    struct scripting_ops *ops)
> +static int list_available_languages_cb(struct scripting_ops *ops, const char *spec)
>  {
> -	struct script_spec *s = malloc(sizeof(*s) + strlen(spec) + 1);
> -
> -	if (s != NULL) {
> -		strcpy(s->spec, spec);
> -		s->ops = ops;
> -	}
> -
> -	return s;
> -}
> -
> -static void script_spec__add(struct script_spec *s)
> -{
> -	list_add_tail(&s->node, &script_specs);
> -}
> -
> -static struct script_spec *script_spec__find(const char *spec)
> -{
> -	struct script_spec *s;
> -
> -	list_for_each_entry(s, &script_specs, node)
> -		if (strcasecmp(s->spec, spec) == 0)
> -			return s;
> -	return NULL;
> -}
> -
> -int script_spec_register(const char *spec, struct scripting_ops *ops)
> -{
> -	struct script_spec *s;
> -
> -	s = script_spec__find(spec);
> -	if (s)
> -		return -1;
> -
> -	s = script_spec__new(spec, ops);
> -	if (!s)
> -		return -1;
> -	else
> -		script_spec__add(s);
> -
> +	fprintf(stderr, "  %-42s [%s]\n", spec, ops->name);
>  	return 0;
>  }
>  
> -static struct scripting_ops *script_spec__lookup(const char *spec)
> -{
> -	struct script_spec *s = script_spec__find(spec);
> -	if (!s)
> -		return NULL;
> -
> -	return s->ops;
> -}
> -
>  static void list_available_languages(void)
>  {
> -	struct script_spec *s;
> -
>  	fprintf(stderr, "\n");
>  	fprintf(stderr, "Scripting language extensions (used in "
>  		"perf script -s [spec:]script.[spec]):\n\n");
> -
> -	list_for_each_entry(s, &script_specs, node)
> -		fprintf(stderr, "  %-42s [%s]\n", s->spec, s->ops->name);
> -
> +	script_spec__for_each(&list_available_languages_cb);
>  	fprintf(stderr, "\n");
>  }
>  
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 0fa8e27769be..3d938fe2de6a 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -1305,11 +1305,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 script_spec_register(const char *spec __maybe_unused, struct scripting_ops *ops __maybe_unused)
> -{
> -	return -1;
> -}
> -
>  arch_syscalls__strerrno_t *arch_syscalls__strerrno_function(const char *arch __maybe_unused)
>  {
>  	return NULL;
> diff --git a/tools/perf/util/trace-event-scripting.c b/tools/perf/util/trace-event-scripting.c
> index 8abb7a7b6888..ad62d8e5a368 100644
> --- a/tools/perf/util/trace-event-scripting.c
> +++ b/tools/perf/util/trace-event-scripting.c
> @@ -24,6 +24,81 @@ unsigned int scripting_max_stack = PERF_MAX_STACK_DEPTH;
>  
>  struct scripting_context *scripting_context;
>  
> +struct script_spec {
> +	struct list_head	node;
> +	struct scripting_ops	*ops;
> +	char			spec[];
> +};
> +
> +static LIST_HEAD(script_specs);
> +
> +static struct script_spec *script_spec__new(const char *spec,
> +					    struct scripting_ops *ops)
> +{
> +	struct script_spec *s = malloc(sizeof(*s) + strlen(spec) + 1);
> +
> +	if (s != NULL) {
> +		strcpy(s->spec, spec);
> +		s->ops = ops;
> +	}
> +
> +	return s;
> +}
> +
> +static void script_spec__add(struct script_spec *s)
> +{
> +	list_add_tail(&s->node, &script_specs);
> +}
> +
> +static struct script_spec *script_spec__find(const char *spec)
> +{
> +	struct script_spec *s;
> +
> +	list_for_each_entry(s, &script_specs, node)
> +		if (strcasecmp(s->spec, spec) == 0)
> +			return s;
> +	return NULL;
> +}
> +
> +static int script_spec_register(const char *spec, struct scripting_ops *ops)
> +{
> +	struct script_spec *s;
> +
> +	s = script_spec__find(spec);
> +	if (s)
> +		return -1;
> +
> +	s = script_spec__new(spec, ops);
> +	if (!s)
> +		return -1;
> +
> +	script_spec__add(s);
> +	return 0;
> +}
> +
> +struct scripting_ops *script_spec__lookup(const char *spec)
> +{
> +	struct script_spec *s = script_spec__find(spec);
> +
> +	if (!s)
> +		return NULL;
> +
> +	return s->ops;
> +}
> +
> +int script_spec__for_each(int (*cb)(struct scripting_ops *ops, const char *spec))
> +{
> +	struct script_spec *s;
> +	int ret = 0;
> +
> +	list_for_each_entry(s, &script_specs, node) {
> +		ret = cb(s->ops, s->spec);
> +		if (ret)
> +			break;
> +	}
> +	return ret;
> +}
> +
>  void scripting_context__update(struct scripting_context *c,
>  			       union perf_event *event,
>  			       struct perf_sample *sample,
> diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
> index bbf8b26bc8da..81fceaf297ba 100644
> --- a/tools/perf/util/trace-event.h
> +++ b/tools/perf/util/trace-event.h
> @@ -116,7 +116,8 @@ struct scripting_ops {
>  
>  extern unsigned int scripting_max_stack;
>  
> -int script_spec_register(const char *spec, struct scripting_ops *ops);
> +struct scripting_ops *script_spec__lookup(const char *spec);
> +int script_spec__for_each(int (*cb)(struct scripting_ops *ops, const char *spec));
>  
>  void script_fetch_insn(struct perf_sample *sample, struct thread *thread,
>  		       struct machine *machine);
> -- 
> 2.47.0.163.g1226f6d8fa-goog