linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND 1/3] perf sort: Drop ip_[lr] arguments from _sort__sym_cmp()
@ 2013-02-06  5:57 Namhyung Kim
  2013-02-06  5:57 ` [PATCH 2/3] perf sort: Make setup_sorting returns an error code Namhyung Kim
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Namhyung Kim @ 2013-02-06  5:57 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim,
	Stephane Eranian

From: Namhyung Kim <namhyung.kim@lge.com>

Current _sort__sym_cmp() function is used for comparing symbols
between two hist entries on symbol, symbol_from and symbol_to sort
keys.  Those functions pass addresses of symbols but it's meaningless
since it gets over-written inside of the _sort__sym_cmp function to a
start address of the symbol.  So just get rid of them.

This might cause a difference than prior output for branch stacks
since it seems not using start address of the symbol but branch
address.  However AFAICS it'd be same as it gets overwritten anyway.

Also remove redundant part of code in sort__sym_cmp().

Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 83336610faa9..03cabe5678d0 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -160,9 +160,10 @@ struct sort_entry sort_dso = {
 
 /* --sort symbol */
 
-static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
-			      u64 ip_l, u64 ip_r)
+static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r)
 {
+	u64 ip_l, ip_r;
+
 	if (!sym_l || !sym_r)
 		return cmp_null(sym_l, sym_r);
 
@@ -178,21 +179,10 @@ static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
 static int64_t
 sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
 {
-	u64 ip_l, ip_r;
-
 	if (!left->ms.sym && !right->ms.sym)
 		return right->level - left->level;
 
-	if (!left->ms.sym || !right->ms.sym)
-		return cmp_null(left->ms.sym, right->ms.sym);
-
-	if (left->ms.sym == right->ms.sym)
-		return 0;
-
-	ip_l = left->ms.sym->start;
-	ip_r = right->ms.sym->start;
-
-	return _sort__sym_cmp(left->ms.sym, right->ms.sym, ip_l, ip_r);
+	return _sort__sym_cmp(left->ms.sym, right->ms.sym);
 }
 
 static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
@@ -383,8 +373,7 @@ sort__sym_from_cmp(struct hist_entry *left, struct hist_entry *right)
 	if (!from_l->sym && !from_r->sym)
 		return right->level - left->level;
 
-	return _sort__sym_cmp(from_l->sym, from_r->sym, from_l->addr,
-			     from_r->addr);
+	return _sort__sym_cmp(from_l->sym, from_r->sym);
 }
 
 static int64_t
@@ -396,7 +385,7 @@ sort__sym_to_cmp(struct hist_entry *left, struct hist_entry *right)
 	if (!to_l->sym && !to_r->sym)
 		return right->level - left->level;
 
-	return _sort__sym_cmp(to_l->sym, to_r->sym, to_l->addr, to_r->addr);
+	return _sort__sym_cmp(to_l->sym, to_r->sym);
 }
 
 static int hist_entry__sym_from_snprintf(struct hist_entry *self, char *bf,
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] perf sort: Make setup_sorting returns an error code
  2013-02-06  5:57 [PATCH RESEND 1/3] perf sort: Drop ip_[lr] arguments from _sort__sym_cmp() Namhyung Kim
@ 2013-02-06  5:57 ` Namhyung Kim
  2013-02-06 22:04   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2013-02-06  5:57 ` [PATCH 3/3] perf sort: Check return value of strdup() Namhyung Kim
  2013-02-06 22:03 ` [tip:perf/core] perf sort: Drop ip_[lr] arguments from _sort__sym_cmp() tip-bot for Namhyung Kim
  2 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2013-02-06  5:57 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim,
	Stephane Eranian, Jiri Olsa

From: Namhyung Kim <namhyung.kim@lge.com>

Currently the setup_sorting() is called for parsing sort keys and
exits if it failed to add the sort key.  As it's included in libperf
it'd be better returning an error code rather than exiting application
inside of the library.

Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Stephane Eranian <eranian@google.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-annotate.c |  3 ++-
 tools/perf/builtin-diff.c     |  4 +++-
 tools/perf/builtin-report.c   |  3 ++-
 tools/perf/builtin-top.c      |  3 ++-
 tools/perf/tests/hists_link.c |  3 ++-
 tools/perf/util/sort.c        | 10 ++++++----
 tools/perf/util/sort.h        |  2 +-
 7 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index dc870cf31b79..95a2ad3f043e 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -309,7 +309,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (symbol__init() < 0)
 		return -1;
 
-	setup_sorting(annotate_usage, options);
+	if (setup_sorting() < 0)
+		usage_with_options(annotate_usage, options);
 
 	if (argc) {
 		/*
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 4af0b580b046..d207a97a2db1 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -605,7 +605,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	ui_init();
 
-	setup_sorting(diff_usage, options);
+	if (setup_sorting() < 0)
+		usage_with_options(diff_usage, options);
+
 	setup_pager();
 
 	sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", NULL);
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 0d221870561a..71d0b0e334be 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -741,7 +741,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		perf_hpp__init();
 	}
 
-	setup_sorting(report_usage, options);
+	if (setup_sorting() < 0)
+		usage_with_options(report_usage, options);
 
 	/*
 	 * Only in the newt browser we are doing integrated annotation,
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index f561757b1bfa..72f6eb7b4173 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1129,7 +1129,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (sort_order == default_sort_order)
 		sort_order = "dso,symbol";
 
-	setup_sorting(top_usage, options);
+	if (setup_sorting() < 0)
+		usage_with_options(top_usage, options);
 
 	if (top.use_stdio)
 		use_browser = 0;
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 0afd9223bde7..1be64a6c5daf 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -449,7 +449,8 @@ int test__hists_link(void)
 		goto out;
 
 	/* default sort order (comm,dso,sym) will be used */
-	setup_sorting(NULL, NULL);
+	if (setup_sorting() < 0)
+		goto out;
 
 	machines__init(&machines);
 
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 03cabe5678d0..d8b48827a17e 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -565,23 +565,25 @@ int sort_dimension__add(const char *tok)
 	return -ESRCH;
 }
 
-void setup_sorting(const char * const usagestr[], const struct option *opts)
+int setup_sorting(void)
 {
 	char *tmp, *tok, *str = strdup(sort_order);
+	int ret = 0;
 
 	for (tok = strtok_r(str, ", ", &tmp);
 			tok; tok = strtok_r(NULL, ", ", &tmp)) {
-		int ret = sort_dimension__add(tok);
+		ret = sort_dimension__add(tok);
 		if (ret == -EINVAL) {
 			error("Invalid --sort key: `%s'", tok);
-			usage_with_options(usagestr, opts);
+			break;
 		} else if (ret == -ESRCH) {
 			error("Unknown --sort key: `%s'", tok);
-			usage_with_options(usagestr, opts);
+			break;
 		}
 	}
 
 	free(str);
+	return ret;
 }
 
 void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index e994ad3e9897..b13e56f6ccbe 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -160,7 +160,7 @@ struct sort_entry {
 extern struct sort_entry sort_thread;
 extern struct list_head hist_entry__sort_list;
 
-void setup_sorting(const char * const usagestr[], const struct option *opts);
+int setup_sorting(void);
 extern int sort_dimension__add(const char *);
 void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
 			     const char *list_name, FILE *fp);
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] perf sort: Check return value of strdup()
  2013-02-06  5:57 [PATCH RESEND 1/3] perf sort: Drop ip_[lr] arguments from _sort__sym_cmp() Namhyung Kim
  2013-02-06  5:57 ` [PATCH 2/3] perf sort: Make setup_sorting returns an error code Namhyung Kim
@ 2013-02-06  5:57 ` Namhyung Kim
  2013-02-06 22:05   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2013-02-06 22:03 ` [tip:perf/core] perf sort: Drop ip_[lr] arguments from _sort__sym_cmp() tip-bot for Namhyung Kim
  2 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2013-02-06  5:57 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim,
	Stephane Eranian

From: Namhyung Kim <namhyung.kim@lge.com>

When setup_sorting() is called, 'str' is passed to strtok_r() but it's
not checked to have a valid pointer.  As strtok_r() accepts NULL
pointer on a first argument and use the third argument in that case,
it can cause a trouble since our third argument, tmp, is not
initialized.

Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index d8b48827a17e..d41926cb9e3f 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -570,6 +570,11 @@ int setup_sorting(void)
 	char *tmp, *tok, *str = strdup(sort_order);
 	int ret = 0;
 
+	if (str == NULL) {
+		error("Not enough memory to setup sort keys");
+		return -ENOMEM;
+	}
+
 	for (tok = strtok_r(str, ", ", &tmp);
 			tok; tok = strtok_r(NULL, ", ", &tmp)) {
 		ret = sort_dimension__add(tok);
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:perf/core] perf sort: Drop ip_[lr] arguments from _sort__sym_cmp()
  2013-02-06  5:57 [PATCH RESEND 1/3] perf sort: Drop ip_[lr] arguments from _sort__sym_cmp() Namhyung Kim
  2013-02-06  5:57 ` [PATCH 2/3] perf sort: Make setup_sorting returns an error code Namhyung Kim
  2013-02-06  5:57 ` [PATCH 3/3] perf sort: Check return value of strdup() Namhyung Kim
@ 2013-02-06 22:03 ` tip-bot for Namhyung Kim
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Namhyung Kim @ 2013-02-06 22:03 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, eranian, paulus, hpa, mingo, a.p.zijlstra,
	namhyung.kim, namhyung, jolsa, tglx

Commit-ID:  51f27d1440cede5a413d279a20b38767b6f85097
Gitweb:     http://git.kernel.org/tip/51f27d1440cede5a413d279a20b38767b6f85097
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Wed, 6 Feb 2013 14:57:15 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 6 Feb 2013 18:09:25 -0300

perf sort: Drop ip_[lr] arguments from _sort__sym_cmp()

Current _sort__sym_cmp() function is used for comparing symbols between
two hist entries on symbol, symbol_from and symbol_to sort keys.  Those
functions pass addresses of symbols but it's meaningless since it gets
over-written inside of the _sort__sym_cmp function to a start address of
the symbol.  So just get rid of them.

This might cause a difference than prior output for branch stacks since
it seems not using start address of the symbol but branch address.
However AFAICS it'd be same as it gets overwritten anyway.

Also remove redundant part of code in sort__sym_cmp().

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1360130237-9963-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 8333661..03cabe5 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -160,9 +160,10 @@ struct sort_entry sort_dso = {
 
 /* --sort symbol */
 
-static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
-			      u64 ip_l, u64 ip_r)
+static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r)
 {
+	u64 ip_l, ip_r;
+
 	if (!sym_l || !sym_r)
 		return cmp_null(sym_l, sym_r);
 
@@ -178,21 +179,10 @@ static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
 static int64_t
 sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
 {
-	u64 ip_l, ip_r;
-
 	if (!left->ms.sym && !right->ms.sym)
 		return right->level - left->level;
 
-	if (!left->ms.sym || !right->ms.sym)
-		return cmp_null(left->ms.sym, right->ms.sym);
-
-	if (left->ms.sym == right->ms.sym)
-		return 0;
-
-	ip_l = left->ms.sym->start;
-	ip_r = right->ms.sym->start;
-
-	return _sort__sym_cmp(left->ms.sym, right->ms.sym, ip_l, ip_r);
+	return _sort__sym_cmp(left->ms.sym, right->ms.sym);
 }
 
 static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
@@ -383,8 +373,7 @@ sort__sym_from_cmp(struct hist_entry *left, struct hist_entry *right)
 	if (!from_l->sym && !from_r->sym)
 		return right->level - left->level;
 
-	return _sort__sym_cmp(from_l->sym, from_r->sym, from_l->addr,
-			     from_r->addr);
+	return _sort__sym_cmp(from_l->sym, from_r->sym);
 }
 
 static int64_t
@@ -396,7 +385,7 @@ sort__sym_to_cmp(struct hist_entry *left, struct hist_entry *right)
 	if (!to_l->sym && !to_r->sym)
 		return right->level - left->level;
 
-	return _sort__sym_cmp(to_l->sym, to_r->sym, to_l->addr, to_r->addr);
+	return _sort__sym_cmp(to_l->sym, to_r->sym);
 }
 
 static int hist_entry__sym_from_snprintf(struct hist_entry *self, char *bf,

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:perf/core] perf sort: Make setup_sorting returns an error code
  2013-02-06  5:57 ` [PATCH 2/3] perf sort: Make setup_sorting returns an error code Namhyung Kim
@ 2013-02-06 22:04   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Namhyung Kim @ 2013-02-06 22:04 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, eranian, paulus, hpa, mingo, a.p.zijlstra,
	acme, namhyung.kim, namhyung, jolsa, tglx

Commit-ID:  553099857702bb77e541c47bde47f6863834d2e2
Gitweb:     http://git.kernel.org/tip/553099857702bb77e541c47bde47f6863834d2e2
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Wed, 6 Feb 2013 14:57:16 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 6 Feb 2013 18:09:26 -0300

perf sort: Make setup_sorting returns an error code

Currently the setup_sorting() is called for parsing sort keys and exits
if it failed to add the sort key.  As it's included in libperf it'd be
better returning an error code rather than exiting application inside of
the library.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1360130237-9963-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c |  3 ++-
 tools/perf/builtin-diff.c     |  4 +++-
 tools/perf/builtin-report.c   |  3 ++-
 tools/perf/builtin-top.c      |  3 ++-
 tools/perf/tests/hists_link.c |  3 ++-
 tools/perf/util/sort.c        | 10 ++++++----
 tools/perf/util/sort.h        |  2 +-
 7 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index dc870cf..95a2ad3 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -309,7 +309,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (symbol__init() < 0)
 		return -1;
 
-	setup_sorting(annotate_usage, options);
+	if (setup_sorting() < 0)
+		usage_with_options(annotate_usage, options);
 
 	if (argc) {
 		/*
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 4af0b58..d207a97 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -605,7 +605,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	ui_init();
 
-	setup_sorting(diff_usage, options);
+	if (setup_sorting() < 0)
+		usage_with_options(diff_usage, options);
+
 	setup_pager();
 
 	sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", NULL);
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 91555d4..96b5a7f 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -751,7 +751,8 @@ repeat:
 
 	}
 
-	setup_sorting(report_usage, options);
+	if (setup_sorting() < 0)
+		usage_with_options(report_usage, options);
 
 	/*
 	 * Only in the newt browser we are doing integrated annotation,
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index f561757..72f6eb7 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1129,7 +1129,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (sort_order == default_sort_order)
 		sort_order = "dso,symbol";
 
-	setup_sorting(top_usage, options);
+	if (setup_sorting() < 0)
+		usage_with_options(top_usage, options);
 
 	if (top.use_stdio)
 		use_browser = 0;
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 0afd922..1be64a6 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -449,7 +449,8 @@ int test__hists_link(void)
 		goto out;
 
 	/* default sort order (comm,dso,sym) will be used */
-	setup_sorting(NULL, NULL);
+	if (setup_sorting() < 0)
+		goto out;
 
 	machines__init(&machines);
 
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 03cabe5..d8b4882 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -565,23 +565,25 @@ int sort_dimension__add(const char *tok)
 	return -ESRCH;
 }
 
-void setup_sorting(const char * const usagestr[], const struct option *opts)
+int setup_sorting(void)
 {
 	char *tmp, *tok, *str = strdup(sort_order);
+	int ret = 0;
 
 	for (tok = strtok_r(str, ", ", &tmp);
 			tok; tok = strtok_r(NULL, ", ", &tmp)) {
-		int ret = sort_dimension__add(tok);
+		ret = sort_dimension__add(tok);
 		if (ret == -EINVAL) {
 			error("Invalid --sort key: `%s'", tok);
-			usage_with_options(usagestr, opts);
+			break;
 		} else if (ret == -ESRCH) {
 			error("Unknown --sort key: `%s'", tok);
-			usage_with_options(usagestr, opts);
+			break;
 		}
 	}
 
 	free(str);
+	return ret;
 }
 
 void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index e994ad3e..b13e56f6 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -160,7 +160,7 @@ struct sort_entry {
 extern struct sort_entry sort_thread;
 extern struct list_head hist_entry__sort_list;
 
-void setup_sorting(const char * const usagestr[], const struct option *opts);
+int setup_sorting(void);
 extern int sort_dimension__add(const char *);
 void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
 			     const char *list_name, FILE *fp);

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:perf/core] perf sort: Check return value of strdup()
  2013-02-06  5:57 ` [PATCH 3/3] perf sort: Check return value of strdup() Namhyung Kim
@ 2013-02-06 22:05   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Namhyung Kim @ 2013-02-06 22:05 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, eranian, paulus, hpa, mingo, a.p.zijlstra,
	namhyung.kim, namhyung, jolsa, tglx

Commit-ID:  5936f54d6ca2857d81188dcdff8c61b8fc482f53
Gitweb:     http://git.kernel.org/tip/5936f54d6ca2857d81188dcdff8c61b8fc482f53
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Wed, 6 Feb 2013 14:57:17 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 6 Feb 2013 18:09:26 -0300

perf sort: Check return value of strdup()

When setup_sorting() is called, 'str' is passed to strtok_r() but it's
not checked to have a valid pointer.  As strtok_r() accepts NULL pointer
on a first argument and use the third argument in that case, it can
cause a trouble since our third argument, tmp, is not initialized.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1360130237-9963-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index d8b4882..d41926c 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -570,6 +570,11 @@ int setup_sorting(void)
 	char *tmp, *tok, *str = strdup(sort_order);
 	int ret = 0;
 
+	if (str == NULL) {
+		error("Not enough memory to setup sort keys");
+		return -ENOMEM;
+	}
+
 	for (tok = strtok_r(str, ", ", &tmp);
 			tok; tok = strtok_r(NULL, ", ", &tmp)) {
 		ret = sort_dimension__add(tok);

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-02-06 22:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-06  5:57 [PATCH RESEND 1/3] perf sort: Drop ip_[lr] arguments from _sort__sym_cmp() Namhyung Kim
2013-02-06  5:57 ` [PATCH 2/3] perf sort: Make setup_sorting returns an error code Namhyung Kim
2013-02-06 22:04   ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-02-06  5:57 ` [PATCH 3/3] perf sort: Check return value of strdup() Namhyung Kim
2013-02-06 22:05   ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-02-06 22:03 ` [tip:perf/core] perf sort: Drop ip_[lr] arguments from _sort__sym_cmp() tip-bot for Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).