Re: [PATCH] tools/lib/traceevent/event-parse: delete pevent_register_function

[Jiri Olsa]

On Mon, Jan 15, 2018 at 12:50:14PM +0800, Wang YanQing wrote:
> After commit 4263cece22e3da94f16fbbcf71ce3807946d3ef3
> ("perf tools: Stop reading the kallsyms data from perf.data"),
> there is no users of pevent_register_function in tree, so we
> could just delete it.
> 

I don't think you can remove this function,
perf is not the only user there

adding Steven to the loop

jirka


> Signed-off-by: Wang YanQing <udknight@xxxxxxxxx>
> ---
>  tools/lib/traceevent/event-parse.c | 183 +------------------------------------
>  tools/lib/traceevent/event-parse.h |   3 -
>  2 files changed, 1 insertion(+), 185 deletions(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index 7ce724f..0d125a3 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -347,96 +347,6 @@ struct func_list {
>  	char			*mod;
>  };
>  
> -static int func_cmp(const void *a, const void *b)
> -{
> -	const struct func_map *fa = a;
> -	const struct func_map *fb = b;
> -
> -	if (fa->addr < fb->addr)
> -		return -1;
> -	if (fa->addr > fb->addr)
> -		return 1;
> -
> -	return 0;
> -}
> -
> -/*
> - * We are searching for a record in between, not an exact
> - * match.
> - */
> -static int func_bcmp(const void *a, const void *b)
> -{
> -	const struct func_map *fa = a;
> -	const struct func_map *fb = b;
> -
> -	if ((fa->addr == fb->addr) ||
> -
> -	    (fa->addr > fb->addr &&
> -	     fa->addr < (fb+1)->addr))
> -		return 0;
> -
> -	if (fa->addr < fb->addr)
> -		return -1;
> -
> -	return 1;
> -}
> -
> -static int func_map_init(struct pevent *pevent)
> -{
> -	struct func_list *funclist;
> -	struct func_list *item;
> -	struct func_map *func_map;
> -	int i;
> -
> -	func_map = malloc(sizeof(*func_map) * (pevent->func_count + 1));
> -	if (!func_map)
> -		return -1;
> -
> -	funclist = pevent->funclist;
> -
> -	i = 0;
> -	while (funclist) {
> -		func_map[i].func = funclist->func;
> -		func_map[i].addr = funclist->addr;
> -		func_map[i].mod = funclist->mod;
> -		i++;
> -		item = funclist;
> -		funclist = funclist->next;
> -		free(item);
> -	}
> -
> -	qsort(func_map, pevent->func_count, sizeof(*func_map), func_cmp);
> -
> -	/*
> -	 * Add a special record at the end.
> -	 */
> -	func_map[pevent->func_count].func = NULL;
> -	func_map[pevent->func_count].addr = 0;
> -	func_map[pevent->func_count].mod = NULL;
> -
> -	pevent->func_map = func_map;
> -	pevent->funclist = NULL;
> -
> -	return 0;
> -}
> -
> -static struct func_map *
> -__find_func(struct pevent *pevent, unsigned long long addr)
> -{
> -	struct func_map *func;
> -	struct func_map key;
> -
> -	if (!pevent->func_map)
> -		func_map_init(pevent);
> -
> -	key.addr = addr;
> -
> -	func = bsearch(&key, pevent->func_map, pevent->func_count,
> -		       sizeof(*pevent->func_map), func_bcmp);
> -
> -	return func;
> -}
> -
>  struct func_resolver {
>  	pevent_func_resolver_t *func;
>  	void		       *priv;
> @@ -448,10 +358,6 @@ struct func_resolver {
>   * @pevent: handle for the pevent
>   * @resolver: function to be used
>   * @priv: resolver function private state.
> - *
> - * Some tools may have already a way to resolve kernel functions, allow them to
> - * keep using it instead of duplicating all the entries inside
> - * pevent->funclist.
>   */
>  int pevent_set_function_resolver(struct pevent *pevent,
>  				 pevent_func_resolver_t *func, void *priv)
> @@ -489,7 +395,7 @@ void pevent_reset_function_resolver(struct pevent *pevent)
>  	struct func_map *map;
>  
>  	if (!pevent->func_resolver)
> -		return __find_func(pevent, addr);
> +		return NULL;
>  
>  	map = &pevent->func_resolver->map;
>  	map->mod  = NULL;
> @@ -543,75 +449,6 @@ const char *pevent_find_function(struct pevent *pevent, unsigned long long addr)
>  	return map->addr;
>  }
>  
> -/**
> - * pevent_register_function - register a function with a given address
> - * @pevent: handle for the pevent
> - * @function: the function name to register
> - * @addr: the address the function starts at
> - * @mod: the kernel module the function may be in (NULL for none)
> - *
> - * This registers a function name with an address and module.
> - * The @func passed in is duplicated.
> - */
> -int pevent_register_function(struct pevent *pevent, char *func,
> -			     unsigned long long addr, char *mod)
> -{
> -	struct func_list *item = malloc(sizeof(*item));
> -
> -	if (!item)
> -		return -1;
> -
> -	item->next = pevent->funclist;
> -	item->func = strdup(func);
> -	if (!item->func)
> -		goto out_free;
> -
> -	if (mod) {
> -		item->mod = strdup(mod);
> -		if (!item->mod)
> -			goto out_free_func;
> -	} else
> -		item->mod = NULL;
> -	item->addr = addr;
> -
> -	pevent->funclist = item;
> -	pevent->func_count++;
> -
> -	return 0;
> -
> -out_free_func:
> -	free(item->func);
> -	item->func = NULL;
> -out_free:
> -	free(item);
> -	errno = ENOMEM;
> -	return -1;
> -}
> -
> -/**
> - * pevent_print_funcs - print out the stored functions
> - * @pevent: handle for the pevent
> - *
> - * This prints out the stored functions.
> - */
> -void pevent_print_funcs(struct pevent *pevent)
> -{
> -	int i;
> -
> -	if (!pevent->func_map)
> -		func_map_init(pevent);
> -
> -	for (i = 0; i < (int)pevent->func_count; i++) {
> -		printf("%016llx %s",
> -		       pevent->func_map[i].addr,
> -		       pevent->func_map[i].func);
> -		if (pevent->func_map[i].mod)
> -			printf(" [%s]\n", pevent->func_map[i].mod);
> -		else
> -			printf("\n");
> -	}
> -}
> -
>  struct printk_map {
>  	unsigned long long		addr;
>  	char				*printk;
> @@ -6777,7 +6614,6 @@ void pevent_free_format(struct event_format *event)
>  void pevent_free(struct pevent *pevent)
>  {
>  	struct cmdline_list *cmdlist, *cmdnext;
> -	struct func_list *funclist, *funcnext;
>  	struct printk_list *printklist, *printknext;
>  	struct pevent_function_handler *func_handler;
>  	struct event_handler *handle;
> @@ -6787,7 +6623,6 @@ void pevent_free(struct pevent *pevent)
>  		return;
>  
>  	cmdlist = pevent->cmdlist;
> -	funclist = pevent->funclist;
>  	printklist = pevent->printklist;
>  
>  	pevent->ref_count--;
> @@ -6807,22 +6642,6 @@ void pevent_free(struct pevent *pevent)
>  		cmdlist = cmdnext;
>  	}
>  
> -	if (pevent->func_map) {
> -		for (i = 0; i < (int)pevent->func_count; i++) {
> -			free(pevent->func_map[i].func);
> -			free(pevent->func_map[i].mod);
> -		}
> -		free(pevent->func_map);
> -	}
> -
> -	while (funclist) {
> -		funcnext = funclist->next;
> -		free(funclist->func);
> -		free(funclist->mod);
> -		free(funclist);
> -		funclist = funcnext;
> -	}
> -
>  	while (pevent->func_handlers) {
>  		func_handler = pevent->func_handlers;
>  		pevent->func_handlers = func_handler->next;
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index 0c03538..af7a693 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -483,10 +483,7 @@ struct pevent {
>  	struct cmdline_list *cmdlist;
>  	int cmdline_count;
>  
> -	struct func_map *func_map;
>  	struct func_resolver *func_resolver;
> -	struct func_list *funclist;
> -	unsigned int func_count;
>  
>  	struct printk_map *printk_map;
>  	struct printk_list *printklist;
> -- 
> 1.8.5.6.2.g3d8a54e.dirty