From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751645AbbCUSVF (ORCPT ); Sat, 21 Mar 2015 14:21:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42425 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751530AbbCUSVC (ORCPT ); Sat, 21 Mar 2015 14:21:02 -0400 Date: Sat, 21 Mar 2015 15:20:43 -0300 From: Arnaldo Carvalho de Melo To: Jiri Olsa Cc: linux-kernel@vger.kernel.org, Adrian Hunter , Corey Ashford , David Ahern , Frederic Weisbecker , Ingo Molnar , Namhyung Kim , Paul Mackerras , Peter Zijlstra , Stephane Eranian , acme@kernel.org Subject: Re: [PATCH 08/14] perf tools: Use kmod_path__parse in map_groups__set_modules_path_dir Message-ID: <20150321182042.GB3404@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1425640442-16048-9-git-send-email-jolsa@kernel.org> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.20 (2009-12-10) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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