From 6ef81c55a2b6584cb642917f5fdf3632ef44b670 Mon Sep 17 00:00:00 2001 From: Mamatha Inamdar Date: Thu, 22 Aug 2019 12:50:49 +0530 Subject: [PATCH] perf session: Return error code for perf_session__new() function on failure This patch is to return error code of perf_new_session function on failure instead of NULL. Test Results: Before Fix: $ perf c2c report -input failed to open nput: No such file or directory $ echo $? 0 $ After Fix: $ perf c2c report -input failed to open nput: No such file or directory $ echo $? 254 $ Committer notes: Fix 'perf tests topology' case, where we use that TEST_ASSERT_VAL(..., session), i.e. we need to pass zero in case of failure, which was the case before when NULL was returned by perf_session__new() for failure, but now we need to negate the result of IS_ERR(session) to respect that TEST_ASSERT_VAL) expectation of zero meaning failure. Reported-by: Nageswara R Sastry Signed-off-by: Mamatha Inamdar Tested-by: Arnaldo Carvalho de Melo Tested-by: Nageswara R Sastry Acked-by: Ravi Bangoria Reviewed-by: Jiri Olsa Reviewed-by: Mukesh Ojha Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Alexey Budankov Cc: Greg Kroah-Hartman Cc: Jeremie Galarneau Cc: Kate Stewart Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Shawn Landden Cc: Song Liu Cc: Thomas Gleixner Cc: Tzvetomir Stoyanov Link: http://lore.kernel.org/lkml/20190822071223.17892.45782.stgit@localhost.localdomain Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-annotate.c | 5 +++-- tools/perf/builtin-buildid-cache.c | 5 +++-- tools/perf/builtin-buildid-list.c | 5 +++-- tools/perf/builtin-c2c.c | 6 ++++-- tools/perf/builtin-diff.c | 9 +++++---- tools/perf/builtin-evlist.c | 5 +++-- tools/perf/builtin-inject.c | 5 +++-- tools/perf/builtin-kmem.c | 5 +++-- tools/perf/builtin-kvm.c | 9 +++++---- tools/perf/builtin-lock.c | 5 +++-- tools/perf/builtin-mem.c | 5 +++-- tools/perf/builtin-record.c | 5 +++-- tools/perf/builtin-report.c | 4 ++-- tools/perf/builtin-sched.c | 11 ++++++----- tools/perf/builtin-script.c | 9 +++++---- tools/perf/builtin-stat.c | 11 ++++++----- tools/perf/builtin-timechart.c | 5 +++-- tools/perf/builtin-top.c | 5 +++-- tools/perf/builtin-trace.c | 4 ++-- tools/perf/tests/topology.c | 5 +++-- tools/perf/util/data-convert-bt.c | 5 ++++- tools/perf/util/session.c | 15 +++++++++++---- 22 files changed, 86 insertions(+), 57 deletions(-) diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 553c651fa0a2..8db8fc9bddef 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -40,6 +40,7 @@ #include #include #include +#include struct perf_annotate { struct perf_tool tool; @@ -584,8 +585,8 @@ int cmd_annotate(int argc, const char **argv) data.path = input_name; annotate.session = perf_session__new(&data, false, &annotate.tool); - if (annotate.session == NULL) - return -1; + if (IS_ERR(annotate.session)) + return PTR_ERR(annotate.session); annotate.has_br_stack = perf_header__has_feat(&annotate.session->header, HEADER_BRANCH_STACK); diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c index 1a69eb565dc0..39efa51d7fb3 100644 --- a/tools/perf/builtin-buildid-cache.c +++ b/tools/perf/builtin-buildid-cache.c @@ -28,6 +28,7 @@ #include "util/util.h" #include "util/probe-file.h" #include +#include static int build_id_cache__kcore_buildid(const char *proc_dir, char *sbuildid) { @@ -422,8 +423,8 @@ int cmd_buildid_cache(int argc, const char **argv) data.force = force; session = perf_session__new(&data, false, NULL); - if (session == NULL) - return -1; + if (IS_ERR(session)) + return PTR_ERR(session); } if (symbol__init(session ? &session->header.env : NULL) < 0) diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c index 5a0d8b378cb5..e3ef75583514 100644 --- a/tools/perf/builtin-buildid-list.c +++ b/tools/perf/builtin-buildid-list.c @@ -18,6 +18,7 @@ #include "util/symbol.h" #include "util/data.h" #include +#include static int sysfs__fprintf_build_id(FILE *fp) { @@ -65,8 +66,8 @@ static int perf_session__list_build_ids(bool force, bool with_hits) goto out; session = perf_session__new(&data, false, &build_id__mark_dso_hit_ops); - if (session == NULL) - return -1; + if (IS_ERR(session)) + return PTR_ERR(session); /* * We take all buildids when the file contains AUX area tracing data diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c index 61aaacc2aedd..3542b6ab9813 100644 --- a/tools/perf/builtin-c2c.c +++ b/tools/perf/builtin-c2c.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -2781,8 +2782,9 @@ static int perf_c2c__report(int argc, const char **argv) } session = perf_session__new(&data, 0, &c2c.tool); - if (session == NULL) { - pr_debug("No memory for session\n"); + if (IS_ERR(session)) { + err = PTR_ERR(session); + pr_debug("Error creating perf session\n"); goto out; } diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index 827e4800d862..c37a78677955 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -23,6 +23,7 @@ #include "util/time-utils.h" #include "util/annotate.h" #include "util/map.h" +#include #include #include #include @@ -1153,9 +1154,9 @@ static int check_file_brstack(void) data__for_each_file(i, d) { d->session = perf_session__new(&d->data, false, &pdiff.tool); - if (!d->session) { + if (IS_ERR(d->session)) { pr_err("Failed to open %s\n", d->data.path); - return -1; + return PTR_ERR(d->session); } has_br_stack = perf_header__has_feat(&d->session->header, @@ -1185,9 +1186,9 @@ static int __cmd_diff(void) data__for_each_file(i, d) { d->session = perf_session__new(&d->data, false, &pdiff.tool); - if (!d->session) { + if (IS_ERR(d->session)) { + ret = PTR_ERR(d->session); pr_err("Failed to open %s\n", d->data.path); - ret = -1; goto out_delete; } diff --git a/tools/perf/builtin-evlist.c b/tools/perf/builtin-evlist.c index 294494e60e48..60509ce4dd28 100644 --- a/tools/perf/builtin-evlist.c +++ b/tools/perf/builtin-evlist.c @@ -15,6 +15,7 @@ #include "util/session.h" #include "util/data.h" #include "util/debug.h" +#include static int __cmd_evlist(const char *file_name, struct perf_attr_details *details) { @@ -28,8 +29,8 @@ static int __cmd_evlist(const char *file_name, struct perf_attr_details *details bool has_tracepoint = false; session = perf_session__new(&data, 0, NULL); - if (session == NULL) - return -1; + if (IS_ERR(session)) + return PTR_ERR(session); evlist__for_each_entry(session->evlist, pos) { perf_evsel__fprintf(pos, details, stdout); diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c index 23a76cf3846f..372ecb3e2c06 100644 --- a/tools/perf/builtin-inject.c +++ b/tools/perf/builtin-inject.c @@ -23,6 +23,7 @@ #include "util/symbol.h" #include "util/synthetic-events.h" #include "util/thread.h" +#include #include @@ -835,8 +836,8 @@ int cmd_inject(int argc, const char **argv) data.path = inject.input_name; inject.session = perf_session__new(&data, true, &inject.tool); - if (inject.session == NULL) - return -1; + if (IS_ERR(inject.session)) + return PTR_ERR(inject.session); if (zstd_init(&(inject.session->zstd_data), 0) < 0) pr_warning("Decompression initialization failed.\n"); diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c index b5682beaad72..1e61e353f579 100644 --- a/tools/perf/builtin-kmem.c +++ b/tools/perf/builtin-kmem.c @@ -14,6 +14,7 @@ #include "util/tool.h" #include "util/callchain.h" #include "util/time-utils.h" +#include #include #include @@ -1956,8 +1957,8 @@ int cmd_kmem(int argc, const char **argv) data.path = input_name; kmem_session = session = perf_session__new(&data, false, &perf_kmem); - if (session == NULL) - return -1; + if (IS_ERR(session)) + return PTR_ERR(session); ret = -1; diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c index 6e3e36658900..c4b22e1b0a40 100644 --- a/tools/perf/builtin-kvm.c +++ b/tools/perf/builtin-kvm.c @@ -33,6 +33,7 @@ #include #include +#include #include #include #include @@ -1091,9 +1092,9 @@ static int read_events(struct perf_kvm_stat *kvm) kvm->tool = eops; kvm->session = perf_session__new(&file, false, &kvm->tool); - if (!kvm->session) { + if (IS_ERR(kvm->session)) { pr_err("Initializing perf session failed\n"); - return -1; + return PTR_ERR(kvm->session); } symbol__init(&kvm->session->header.env); @@ -1446,8 +1447,8 @@ static int kvm_events_live(struct perf_kvm_stat *kvm, * perf session */ kvm->session = perf_session__new(&data, false, &kvm->tool); - if (kvm->session == NULL) { - err = -1; + if (IS_ERR(kvm->session)) { + err = PTR_ERR(kvm->session); goto out; } kvm->session->evlist = kvm->evlist; diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c index 4c2b7f437cdf..474dfd59d7eb 100644 --- a/tools/perf/builtin-lock.c +++ b/tools/perf/builtin-lock.c @@ -30,6 +30,7 @@ #include #include #include +#include static struct perf_session *session; @@ -872,9 +873,9 @@ static int __cmd_report(bool display_info) }; session = perf_session__new(&data, false, &eops); - if (!session) { + if (IS_ERR(session)) { pr_err("Initializing perf session failed\n"); - return -1; + return PTR_ERR(session); } symbol__init(&session->header.env); diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c index 27d2bde943a8..a13f5817d6fc 100644 --- a/tools/perf/builtin-mem.c +++ b/tools/perf/builtin-mem.c @@ -17,6 +17,7 @@ #include "util/dso.h" #include "util/map.h" #include "util/symbol.h" +#include #define MEM_OPERATION_LOAD 0x1 #define MEM_OPERATION_STORE 0x2 @@ -249,8 +250,8 @@ static int report_raw_events(struct perf_mem *mem) struct perf_session *session = perf_session__new(&data, false, &mem->tool); - if (session == NULL) - return -1; + if (IS_ERR(session)) + return PTR_ERR(session); if (mem->cpu_list) { ret = perf_session__cpu_bitmap(session, mem->cpu_list, diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 4bd11c918e73..3f66a49a997f 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -54,6 +54,7 @@ #include #include #include +#include #include #include #include @@ -1354,9 +1355,9 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) } session = perf_session__new(data, false, tool); - if (session == NULL) { + if (IS_ERR(session)) { pr_err("Perf session creation failed.\n"); - return -1; + return PTR_ERR(session); } fd = perf_data__fd(data); diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 3047e5169d9d..aae0e57c60fb 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -1269,8 +1269,8 @@ int cmd_report(int argc, const char **argv) repeat: session = perf_session__new(&data, false, &report.tool); - if (session == NULL) - return -1; + if (IS_ERR(session)) + return PTR_ERR(session); ret = evswitch__init(&report.evswitch, session->evlist, stderr); if (ret) diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c index f0b828c201cc..079e67a36904 100644 --- a/tools/perf/builtin-sched.c +++ b/tools/perf/builtin-sched.c @@ -40,6 +40,7 @@ #include #include #include +#include #include @@ -1797,9 +1798,9 @@ static int perf_sched__read_events(struct perf_sched *sched) int rc = -1; session = perf_session__new(&data, false, &sched->tool); - if (session == NULL) { - pr_debug("No Memory for session\n"); - return -1; + if (IS_ERR(session)) { + pr_debug("Error creating perf session"); + return PTR_ERR(session); } symbol__init(&session->header.env); @@ -2989,8 +2990,8 @@ static int perf_sched__timehist(struct perf_sched *sched) symbol_conf.use_callchain = sched->show_callchain; session = perf_session__new(&data, false, &sched->tool); - if (session == NULL) - return -ENOMEM; + if (IS_ERR(session)) + return PTR_ERR(session); evlist = session->evlist; diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c index e079b34201f2..a17a9306bdf6 100644 --- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -52,6 +52,7 @@ #include #include #include +#include #include "util/record.h" #include "util/util.h" #include "perf.h" @@ -3083,8 +3084,8 @@ int find_scripts(char **scripts_array, char **scripts_path_array, int num, int i = 0; session = perf_session__new(&data, false, NULL); - if (!session) - return -1; + if (IS_ERR(session)) + return PTR_ERR(session); snprintf(scripts_path, MAXPATHLEN, "%s/scripts", get_argv_exec_path()); @@ -3754,8 +3755,8 @@ int cmd_script(int argc, const char **argv) } session = perf_session__new(&data, false, &script.tool); - if (session == NULL) - return -1; + if (IS_ERR(session)) + return PTR_ERR(session); if (header || header_only) { script.tool.show_feat_hdr = SHOW_FEAT_HEADER; diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 60cdd383af81..f7d13326b830 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -83,6 +83,7 @@ #include #include #include +#include #include #include @@ -1436,9 +1437,9 @@ static int __cmd_record(int argc, const char **argv) } session = perf_session__new(data, false, NULL); - if (session == NULL) { - pr_err("Perf session creation failed.\n"); - return -1; + if (IS_ERR(session)) { + pr_err("Perf session creation failed\n"); + return PTR_ERR(session); } init_features(session); @@ -1635,8 +1636,8 @@ static int __cmd_report(int argc, const char **argv) perf_stat.data.mode = PERF_DATA_MODE_READ; session = perf_session__new(&perf_stat.data, false, &perf_stat.tool); - if (session == NULL) - return -1; + if (IS_ERR(session)) + return PTR_ERR(session); perf_stat.session = session; stat_config.output = stderr; diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c index e0e822695a29..9e84fae9b096 100644 --- a/tools/perf/builtin-timechart.c +++ b/tools/perf/builtin-timechart.c @@ -35,6 +35,7 @@ #include "util/tool.h" #include "util/data.h" #include "util/debug.h" +#include #ifdef LACKS_OPEN_MEMSTREAM_PROTOTYPE FILE *open_memstream(char **ptr, size_t *sizeloc); @@ -1601,8 +1602,8 @@ static int __cmd_timechart(struct timechart *tchart, const char *output_name) &tchart->tool); int ret = -EINVAL; - if (session == NULL) - return -1; + if (IS_ERR(session)) + return PTR_ERR(session); symbol__init(&session->header.env); diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index b052470f89b4..8da3c939e6b0 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -77,6 +77,7 @@ #include #include #include +#include #include @@ -1672,8 +1673,8 @@ int cmd_top(int argc, const char **argv) } top.session = perf_session__new(NULL, false, NULL); - if (top.session == NULL) { - status = -1; + if (IS_ERR(top.session)) { + status = PTR_ERR(top.session); goto out_delete_evlist; } diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index f0f735093e21..a292658b4232 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -3585,8 +3585,8 @@ static int trace__replay(struct trace *trace) trace->multiple_threads = true; session = perf_session__new(&data, false, &trace->tool); - if (session == NULL) - return -1; + if (IS_ERR(session)) + return PTR_ERR(session); if (trace->opts.target.pid) symbol_conf.pid_list_str = strdup(trace->opts.target.pid); diff --git a/tools/perf/tests/topology.c b/tools/perf/tests/topology.c index 7d845d913d7d..4a800499d7c3 100644 --- a/tools/perf/tests/topology.c +++ b/tools/perf/tests/topology.c @@ -8,6 +8,7 @@ #include "session.h" #include "evlist.h" #include "debug.h" +#include #define TEMPL "/tmp/perf-test-XXXXXX" #define DATA_SIZE 10 @@ -39,7 +40,7 @@ static int session_write_header(char *path) }; session = perf_session__new(&data, false, NULL); - TEST_ASSERT_VAL("can't get session", session); + TEST_ASSERT_VAL("can't get session", !IS_ERR(session)); session->evlist = perf_evlist__new_default(); TEST_ASSERT_VAL("can't get evlist", session->evlist); @@ -70,7 +71,7 @@ static int check_cpu_topology(char *path, struct perf_cpu_map *map) int i; session = perf_session__new(&data, false, NULL); - TEST_ASSERT_VAL("can't get session", session); + TEST_ASSERT_VAL("can't get session", !IS_ERR(session)); /* On platforms with large numbers of CPUs process_cpu_topology() * might issue an error while reading the perf.data file section diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c index 0c268449959c..dbc772bfb04e 100644 --- a/tools/perf/util/data-convert-bt.c +++ b/tools/perf/util/data-convert-bt.c @@ -30,6 +30,7 @@ #include "machine.h" #include "config.h" #include +#include #define pr_N(n, fmt, ...) \ eprintf(n, debug_data_convert, fmt, ##__VA_ARGS__) @@ -1619,8 +1620,10 @@ int bt_convert__perf2ctf(const char *input, const char *path, err = -1; /* perf.data session */ session = perf_session__new(&data, 0, &c.tool); - if (!session) + if (IS_ERR(session)) { + err = PTR_ERR(session); goto free_writer; + } if (c.queue_size) { ordered_events__set_alloc_size(&session->ordered_events, diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index 58b5bc34ba12..a621c73bad42 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -34,6 +34,7 @@ #include "../perf.h" #include "arch/common.h" #include +#include #ifdef HAVE_ZSTD_SUPPORT static int perf_session__process_compressed_event(struct perf_session *session, @@ -187,6 +188,7 @@ static int ordered_events__deliver_event(struct ordered_events *oe, struct perf_session *perf_session__new(struct perf_data *data, bool repipe, struct perf_tool *tool) { + int ret = -ENOMEM; struct perf_session *session = zalloc(sizeof(*session)); if (!session) @@ -201,13 +203,15 @@ struct perf_session *perf_session__new(struct perf_data *data, perf_env__init(&session->header.env); if (data) { - if (perf_data__open(data)) + ret = perf_data__open(data); + if (ret < 0) goto out_delete; session->data = data; if (perf_data__is_read(data)) { - if (perf_session__open(session) < 0) + ret = perf_session__open(session); + if (ret < 0) goto out_delete; /* @@ -222,8 +226,11 @@ struct perf_session *perf_session__new(struct perf_data *data, perf_evlist__init_trace_event_sample_raw(session->evlist); /* Open the directory data. */ - if (data->is_dir && perf_data__open_dir(data)) + if (data->is_dir) { + ret = perf_data__open_dir(data); + if (ret) goto out_delete; + } } } else { session->machines.host.env = &perf_env; @@ -256,7 +263,7 @@ struct perf_session *perf_session__new(struct perf_data *data, out_delete: perf_session__delete(session); out: - return NULL; + return ERR_PTR(ret); } static void perf_session__delete_threads(struct perf_session *session) -- 2.30.2