Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754785AbcKYOkc (ORCPT ); Fri, 25 Nov 2016 09:40:32 -0500 Received: from mail.kernel.org ([198.145.29.136]:34796 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753052AbcKYOkW (ORCPT ); Fri, 25 Nov 2016 09:40:22 -0500 Date: Fri, 25 Nov 2016 11:40:02 -0300 From: Arnaldo Carvalho de Melo To: Alexis Berlemont Cc: linux-kernel@vger.kernel.org, peterz@infradead.org, mingo@redhat.com, alexander.shishkin@linux.intel.com, hemant@linux.vnet.ibm.com Subject: Re: [PATCH v2 1/2] perf sdt: add scanning of sdt probles arguments Message-ID: <20161125144002.GC16426@kernel.org> References: <52251cbe-6825-1d03-7b5b-e7517549caa0@linux.vnet.ibm.com> <20161118235637.21651-2-alexis.berlemont@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161118235637.21651-2-alexis.berlemont@gmail.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2532 Lines: 70 Em Sat, Nov 19, 2016 at 12:56:36AM +0100, Alexis Berlemont escreveu: > During a "perf buildid-cache --add" command, the section > ".note.stapsdt" of the "added" binary is scanned in order to list the > available SDT markers available in a binary. The parts containing the > probes arguments were left unscanned. > > The whole section is now parsed; the probe arguments are extracted for > later use. > > Signed-off-by: Alexis Berlemont > --- > tools/perf/util/symbol-elf.c | 16 +++++++++++++++- > tools/perf/util/symbol.h | 1 + > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > index 99400b0..0fbe0b2 100644 > --- a/tools/perf/util/symbol-elf.c > +++ b/tools/perf/util/symbol-elf.c > @@ -1822,7 +1822,7 @@ void kcore_extract__delete(struct kcore_extract *kce) > static int populate_sdt_note(Elf **elf, const char *data, size_t len, > struct list_head *sdt_notes) > { > - const char *provider, *name; > + const char *provider, *name, *args; > struct sdt_note *tmp = NULL; > GElf_Ehdr ehdr; > GElf_Addr base_off = 0; > @@ -1881,6 +1881,20 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len, > goto out_free_prov; > } > > + args = (const char *)memchr(name, '\0', data + len - name); Humm, no need for casting? > + > + /* > + * There is no argument if: > + * - We reached the end of the note; > + * - There is not enough room to hold a potential string; > + * - The argument string is empty or just contains ':'. > + */ > + if (args == NULL || data + len - args < 2 || > + args[1] == ':' || args[1] == '\0') > + tmp->args = NULL; > + else > + tmp->args = strdup(++args); Shouldn't we check this and do error back propagation? I.e. if there are args and we don't handle them, silently, that looks bad > + > if (gelf_getclass(*elf) == ELFCLASS32) { > memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr)); > tmp->bit32 = true; > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h > index 2d0a905..913be07 100644 > --- a/tools/perf/util/symbol.h > +++ b/tools/perf/util/symbol.h > @@ -347,6 +347,7 @@ int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb); > struct sdt_note { > char *name; /* name of the note*/ > char *provider; /* provider name */ > + char *args; > bool bit32; /* whether the location is 32 bits? */ > union { /* location, base and semaphore addrs */ > Elf64_Addr a64[3]; > -- > 2.10.2