[PATCH v2 0/7] perf: Fix error return code

[PATCH v2 0/7] perf: Fix error return code

[Shang XiaoJing]

Set the error return code that has been assigned by previous function to
the default value, which may caused by the patches insert function that
assign to the error code and overwrites the default one.

changes in v2:
- adjust commit msg
- adjust subject
- discard one patch
- add fix msg

Shang XiaoJing (7):
  perf lock: Fix error return code in __cmd_contention()
  perf probe: Fix error return code in perf_del_probe_events()
  perf record: Fix error return code in cmd_record()
  perf report: Fix error return code in cmd_report()
  perf sched: Fix error return code in perf_sched__timehist()
  perf stat: Fix error return code in cmd_stat()
  perf trace: Fix error return code in trace__replay() and cmd_trace()

 tools/perf/builtin-lock.c   | 10 +++++++---
 tools/perf/builtin-probe.c  |  8 ++++++--
 tools/perf/builtin-record.c |  3 +++
 tools/perf/builtin-report.c |  5 ++++-
 tools/perf/builtin-sched.c  |  1 +
 tools/perf/builtin-stat.c   |  1 +
 tools/perf/builtin-trace.c  |  5 +++++
 7 files changed, 27 insertions(+), 6 deletions(-)

-- 
2.17.1

[PATCH v2 1/7] perf lock: Fix error return code in __cmd_contention()

[Shang XiaoJing]

Commit 6fda2405f414 ("perf lock: Implement cpu and task filters for
BPF") inserts code to the __cmd_contention() that assigns to the error
code, but not assigned when lock_contention_prepare failed. Besides, 
the following checks should return the default error code when failed.

BTW, the error code of add_output_field is also changed to ensure
setup_output_field can return specific error code instead of -1.

Fixes: 6fda2405f414 ("perf lock: Implement cpu and task filters for
BPF")
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
changes in v2:
- add fix msg
---
 tools/perf/builtin-lock.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 52a6a10a610c..8c4a641ea9ad 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -335,7 +335,7 @@ static int add_output_field(bool contention, char *name)
 	}
 
 	pr_err("Unknown output field: %s\n", name);
-	return -1;
+	return -EINVAL;
 }
 
 static int setup_output_field(bool contention, const char *str)
@@ -1626,6 +1626,7 @@ static int __cmd_contention(int argc, const char **argv)
 
 		if (lock_contention_prepare(&con) < 0) {
 			pr_err("lock contention BPF setup failed\n");
+			err = -EINVAL;
 			goto out_delete;
 		}
 	} else {
@@ -1645,11 +1646,14 @@ static int __cmd_contention(int argc, const char **argv)
 		}
 	}
 
-	if (setup_output_field(true, output_fields))
+	err = setup_output_field(true, output_fields);
+	if (err < 0)
 		goto out_delete;
 
-	if (select_key(true))
+	if (select_key(true)) {
+		err = -EINVAL;
 		goto out_delete;
+	}
 
 	if (show_thread_stats)
 		aggr_mode = LOCK_AGGR_TASK;
-- 
2.17.1

[PATCH v2 2/7] perf probe: Fix error return code in perf_del_probe_events()

[Shang XiaoJing]

Commit e607f1426b584 ("perf probe: Print deleted events in cmd_probe()")
add perf_del_probe_events(), which returns ret as the error code,
through the correct error code is recorded in ret2. Set the error code in
perf_del_probe_events() as ret2 when it is needed.

Fixes: e607f1426b584 ("perf probe: Print deleted events in cmd_probe()")
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
changes in v2:
- add fix msg
---
 tools/perf/builtin-probe.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index f62298f5db3b..b1a7876d9031 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -465,10 +465,14 @@ static int perf_del_probe_events(struct strfilter *filter)
 			pr_info("Removed event: %s\n", ent->s);
 
 		ret2 = probe_file__del_strlist(ufd, ulist);
-		if (ret2 < 0)
+		if (ret2 < 0) {
+			ret = ret2;
 			goto error;
-	} else if (ret2 == -ENOMEM)
+		}
+	} else if (ret2 == -ENOMEM) {
+		ret = ret2;
 		goto error;
+	}
 
 	if (ret == -ENOENT && ret2 == -ENOENT)
 		pr_warning("\"%s\" does not hit any event.\n", str);
-- 
2.17.1

[PATCH v2 3/7] perf record: Fix error return code in cmd_record()

[Shang XiaoJing]

Commit b5f2511d4b397 ("perf record: Implement compatibility checks")
inserts check functions in cmd_record(), but not assign to the error
code. So does the evlist__fix_hybrid_cpus() added by commit
1d3351e631fc3 ("perf tools: Enable on a list of CPUs for hybrid").
Set the untimely updated error code in cmd_record as -EINVAL when there
encounters the error.

Fixes: b5f2511d4b397 ("perf record: Implement compatibility checks")
Fixes: 1d3351e631fc3 ("perf tools: Enable on a list of CPUs for hybrid")
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
---
changes in v2:
- add fix msg.
---
 tools/perf/builtin-record.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 02e38f50a138..959a639f95f1 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -4015,10 +4015,12 @@ int cmd_record(int argc, const char **argv)
 	if (record__threads_enabled(rec)) {
 		if (rec->opts.affinity != PERF_AFFINITY_SYS) {
 			pr_err("--affinity option is mutually exclusive to parallel streaming mode.\n");
+			err = -EINVAL;
 			goto out_opts;
 		}
 		if (record__aio_enabled(rec)) {
 			pr_err("Asynchronous streaming mode (--aio) is mutually exclusive to parallel streaming mode.\n");
+			err = -EINVAL;
 			goto out_opts;
 		}
 	}
@@ -4161,6 +4163,7 @@ int cmd_record(int argc, const char **argv)
 	if (evlist__fix_hybrid_cpus(rec->evlist, rec->opts.target.cpu_list)) {
 		pr_err("failed to use cpu list %s\n",
 		       rec->opts.target.cpu_list);
+		err = -EINVAL;
 		goto out;
 	}
 
-- 
2.17.1

[PATCH v2 4/7] perf report: Fix error return code in cmd_report()

[Shang XiaoJing]

As many check functions have not assigned to the error code, reset the
error return code in cmd_report as -EINVAL before a series of checks.

Set the error code as -EINVAL when symbol__init() failed.

Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
changes in v2:
- adjust commit msg.
- change the position of resetting the error code from "end of the
evswitch__init()" to "start of a series of checks".
---
 tools/perf/builtin-report.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 8361890176c2..3775e5d0384c 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1508,6 +1508,7 @@ int cmd_report(int argc, const char **argv)
 			sort_order = "srcline,symbol,dso";
 	}
 
+	ret = -EINVAL;
 	if (report.mem_mode) {
 		if (sort__mode == SORT_MODE__BRANCH) {
 			pr_err("branch and mem mode incompatible\n");
@@ -1644,8 +1645,10 @@ int cmd_report(int argc, const char **argv)
 		annotation_config__init(&report.annotation_opts);
 	}
 
-	if (symbol__init(&session->header.env) < 0)
+	if (symbol__init(&session->header.env) < 0) {
+		ret = -EINVAL;
 		goto error;
+	}
 
 	if (report.time_str) {
 		ret = perf_time__parse_for_ranges(report.time_str, session,
-- 
2.17.1

[PATCH v2 5/7] perf sched: Fix error return code in perf_sched__timehist()

[Shang XiaoJing]

Commit c30d630d1bcf ("perf sched timehist: Add support for filtering on
CPU") inserts check function perf_session__cpu_bitmap() in
perf_sched__timehist(), which assigned to the error code but not reset.
Reset the error return code in perf_sched__timehist to -1 after assigned.

Fixes: c30d630d1bcf ("perf sched timehist: Add support for filtering on
CPU")
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
Cc: David Ahern <dsahern@gmail.com>
---
changes in v2:
- add fix msg
---
 tools/perf/builtin-sched.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index f93737eef07b..0c5e28e8251a 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -3048,6 +3048,7 @@ static int perf_sched__timehist(struct perf_sched *sched)
 		err = perf_session__cpu_bitmap(session, cpu_list, cpu_bitmap);
 		if (err < 0)
 			goto out;
+		err = -1;
 	}
 
 	evlist = session->evlist;
-- 
2.17.1

[PATCH v2 6/7] perf stat: Fix error return code in cmd_stat()

[Shang XiaoJing]

Commit f07952b17969 ("perf stat: Basic support for iostat in perf")
inserts check function iostat_prepare(), which assign to the error
code but not reset. Reset the error return value in cmd_stat() as
-EINVAL after assigned.

Fixes: f07952b17969 ("perf stat: Basic support for iostat in perf")
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
Cc: Alexander Antonov <alexander.antonov@linux.intel.com>
---
changes in v2:
- discard meaningless change.
- add fix msg.
---
 tools/perf/builtin-stat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index e05fe72c1d87..bd8a91597227 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2438,6 +2438,7 @@ int cmd_stat(int argc, const char **argv)
 			iostat_list(evsel_list, &stat_config);
 		if (iostat_mode == IOSTAT_RUN && !target__has_cpu(&target))
 			target.system_wide = true;
+		status = -EINVAL;
 	}
 
 	if ((stat_config.aggr_mode == AGGR_THREAD) && (target.system_wide))
-- 
2.17.1

[PATCH v2 7/7] perf trace: Fix error return code in trace__replay() and cmd_trace()

[Shang XiaoJing]

Reset the error return code in trace__replay() to -1 to ensure the
following checks can return correct error code when they failed.

Use PTR_ERR(evsel) as the error code in cmd_trace for more accurate.

Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
changes in v2:
- adjust commit msg.
---
 tools/perf/builtin-trace.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 3ecc31375f90..57a6ba158bf1 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -4277,6 +4277,8 @@ static int trace__replay(struct trace *trace)
 	if (err)
 		goto out;
 
+	err = -1;
+
 	evsel = evlist__find_tracepoint_by_name(session->evlist, "raw_syscalls:sys_enter");
 	trace->syscalls.events.sys_enter = evsel;
 	/* older kernels have syscalls tp versus raw_syscalls */
@@ -4972,9 +4974,12 @@ int cmd_trace(int argc, const char **argv)
 	if (IS_ERR(evsel)) {
 		bpf__strerror_setup_output_event(trace.evlist, PTR_ERR(evsel), bf, sizeof(bf));
 		pr_err("ERROR: Setup trace syscalls enter failed: %s\n", bf);
+		err = PTR_ERR(evsel);
 		goto out;
 	}
 
+	err = -1;
+
 	if (evsel) {
 		trace.syscalls.events.augmented = evsel;
 
-- 
2.17.1