Re: [PATCH 08/14] perf tools: Use kmod_path__parse in map_groups__set_modules_path_dir

[Arnaldo Carvalho de Melo]

Em Fri, Mar 06, 2015 at 12:13:56PM +0100, Jiri Olsa escreveu:
> Replacing the file name parsing with kmod_path__parse
> and moving the dso update into new separate function.
> 
> ---
>  tools/perf/util/machine.c | 65 ++++++++++++++++++++++++++++++-----------------
>  1 file changed, 41 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c

> +static int map_groups__set_module_path(struct map_groups *mg, const char *path,
> +				       struct kmod_path *m)
> +{
> +	struct map *map;
> +	char *long_name;
> +
> +	map = map_groups__find_by_name(mg, MAP__FUNCTION, m->name);
> +	if (map == NULL)
> +		return 0;
> +
> +	long_name = strdup(path);
> +	if (long_name == NULL)
> +		return -ENOMEM;
> +
> +	dso__set_long_name(map->dso, long_name, true);
> +	dso__kernel_module_get_build_id(map->dso, "");
> +
> +	/*
> +	 * Full name could reveal us kmod compression, so
> +	 * we need to update the symtab_type if needed.
> +	 */
> +	if (m->comp && is_kmod_dso(map->dso))
> +		map->dso->symtab_type++;

Ouch, does this assumes this is a:

DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE

And that, because in the dso_binary_type enum
DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP comes right after it, and thus
this cryptic increment is equivalent to:

		map->dso->symtab_type = DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP;

?

Also, this part:

----
    Replacing the file name parsing with kmod_path__parse and moving the
dso update into new separate function.
----

Indicates that this probably would be best done in two separate patches,
one that introduces the new function, but is equivalent to the existing
code, and the next one adding the new logic?

Processed all the patches up to this one, stopping now ;-)

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/