The code contains some technical debt in error handling, which complicates the consolidation of duplicated code.
Introduce an fatal() function to replace the common pattern of err_msg() followed by exit(EXIT_FAILURE), reducing the length of an already long function. Further patches using fatal() follow. Signed-off-by: Costa Shulyupin <costa.s...@redhat.com> --- tools/tracing/rtla/src/osnoise_hist.c | 42 ++++++++-------------- tools/tracing/rtla/src/osnoise_top.c | 42 ++++++++-------------- tools/tracing/rtla/src/timerlat_hist.c | 50 +++++++++----------------- tools/tracing/rtla/src/timerlat_top.c | 48 +++++++++---------------- tools/tracing/rtla/src/timerlat_u.c | 12 +++---- tools/tracing/rtla/src/utils.c | 14 ++++++++ tools/tracing/rtla/src/utils.h | 1 + 7 files changed, 80 insertions(+), 129 deletions(-) diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c index dffb6d0a98d7..43c323521f55 100644 --- a/tools/tracing/rtla/src/osnoise_hist.c +++ b/tools/tracing/rtla/src/osnoise_hist.c @@ -592,10 +592,8 @@ static struct common_params break; case 'e': tevent = trace_event_alloc(optarg); - if (!tevent) { - err_msg("Error alloc trace event"); - exit(EXIT_FAILURE); - } + if (!tevent) + fatal("Error alloc trace event"); if (params->common.events) tevent->next = params->common.events; @@ -615,10 +613,8 @@ static struct common_params case 'H': params->common.hk_cpus = 1; retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set); - if (retval) { - err_msg("Error parsing house keeping CPUs\n"); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error parsing house keeping CPUs\n"); break; case 'p': params->period = get_llong_from_str(optarg); @@ -671,10 +667,8 @@ static struct common_params case '4': /* trigger */ if (params->common.events) { retval = trace_event_add_trigger(params->common.events, optarg); - if (retval) { - err_msg("Error adding trigger %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding trigger %s\n", optarg); } else { osnoise_hist_usage("--trigger requires a previous -e\n"); } @@ -682,10 +676,8 @@ static struct common_params case '5': /* filter */ if (params->common.events) { retval = trace_event_add_filter(params->common.events, optarg); - if (retval) { - err_msg("Error adding filter %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding filter %s\n", optarg); } else { osnoise_hist_usage("--filter requires a previous -e\n"); } @@ -699,18 +691,14 @@ static struct common_params case '8': retval = actions_parse(¶ms->common.threshold_actions, optarg, "osnoise_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s\n", optarg); break; case '9': retval = actions_parse(¶ms->common.end_actions, optarg, "osnoise_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s\n", optarg); break; default: osnoise_hist_usage("Invalid option"); @@ -720,10 +708,8 @@ static struct common_params if (trace_output) actions_add_trace_output(¶ms->common.threshold_actions, trace_output); - if (geteuid()) { - err_msg("rtla needs root permission\n"); - exit(EXIT_FAILURE); - } + if (geteuid()) + fatal("rtla needs root permission\n"); if (params->common.hist.no_index && !params->common.hist.with_zeros) osnoise_hist_usage("no-index set and with-zeros not set - it does not make sense"); diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c index 95418f7ecc96..43fb50b5d936 100644 --- a/tools/tracing/rtla/src/osnoise_top.c +++ b/tools/tracing/rtla/src/osnoise_top.c @@ -426,10 +426,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) break; case 'e': tevent = trace_event_alloc(optarg); - if (!tevent) { - err_msg("Error alloc trace event"); - exit(EXIT_FAILURE); - } + if (!tevent) + fatal("Error alloc trace event"); if (params->common.events) tevent->next = params->common.events; @@ -443,10 +441,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case 'H': params->common.hk_cpus = 1; retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set); - if (retval) { - err_msg("Error parsing house keeping CPUs\n"); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error parsing house keeping CPUs\n"); break; case 'p': params->period = get_llong_from_str(optarg); @@ -490,10 +486,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case '0': /* trigger */ if (params->common.events) { retval = trace_event_add_trigger(params->common.events, optarg); - if (retval) { - err_msg("Error adding trigger %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding trigger %s\n", optarg); } else { osnoise_top_usage(params, "--trigger requires a previous -e\n"); } @@ -501,10 +495,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case '1': /* filter */ if (params->common.events) { retval = trace_event_add_filter(params->common.events, optarg); - if (retval) { - err_msg("Error adding filter %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding filter %s\n", optarg); } else { osnoise_top_usage(params, "--filter requires a previous -e\n"); } @@ -518,18 +510,14 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case '4': retval = actions_parse(¶ms->common.threshold_actions, optarg, "osnoise_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s\n", optarg); break; case '5': retval = actions_parse(¶ms->common.end_actions, optarg, "osnoise_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s\n", optarg); break; default: osnoise_top_usage(params, "Invalid option"); @@ -539,10 +527,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) if (trace_output) actions_add_trace_output(¶ms->common.threshold_actions, trace_output); - if (geteuid()) { - err_msg("osnoise needs root permission\n"); - exit(EXIT_FAILURE); - } + if (geteuid()) + fatal("osnoise needs root permission\n"); return ¶ms->common; } diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index 606c1688057b..6504556be5e4 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -913,10 +913,8 @@ static struct common_params break; case 'e': tevent = trace_event_alloc(optarg); - if (!tevent) { - err_msg("Error alloc trace event"); - exit(EXIT_FAILURE); - } + if (!tevent) + fatal("Error alloc trace event"); if (params->common.events) tevent->next = params->common.events; @@ -936,10 +934,8 @@ static struct common_params case 'H': params->common.hk_cpus = 1; retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set); - if (retval) { - err_msg("Error parsing house keeping CPUs\n"); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error parsing house keeping CPUs\n"); break; case 'i': params->common.stop_us = get_llong_from_str(optarg); @@ -1005,10 +1001,8 @@ static struct common_params case '6': /* trigger */ if (params->common.events) { retval = trace_event_add_trigger(params->common.events, optarg); - if (retval) { - err_msg("Error adding trigger %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding trigger %s\n", optarg); } else { timerlat_hist_usage("--trigger requires a previous -e\n"); } @@ -1016,20 +1010,16 @@ static struct common_params case '7': /* filter */ if (params->common.events) { retval = trace_event_add_filter(params->common.events, optarg); - if (retval) { - err_msg("Error adding filter %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding filter %s\n", optarg); } else { timerlat_hist_usage("--filter requires a previous -e\n"); } break; case '8': params->dma_latency = get_llong_from_str(optarg); - if (params->dma_latency < 0 || params->dma_latency > 10000) { - err_msg("--dma-latency needs to be >= 0 and < 10000"); - exit(EXIT_FAILURE); - } + if (params->dma_latency < 0 || params->dma_latency > 10000) + fatal("--dma-latency needs to be >= 0 and < 10000"); break; case '9': params->no_aa = 1; @@ -1049,31 +1039,25 @@ static struct common_params case '\5': retval = actions_parse(¶ms->common.threshold_actions, optarg, "timerlat_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s\n", optarg); break; case '\6': retval = actions_parse(¶ms->common.end_actions, optarg, "timerlat_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s\n", optarg); break; default: - timerlat_hist_usage("Invalid option"); + fatal("Invalid option\n"); } } if (trace_output) actions_add_trace_output(¶ms->common.threshold_actions, trace_output); - if (geteuid()) { - err_msg("rtla needs root permission\n"); - exit(EXIT_FAILURE); - } + if (geteuid()) + fatal("rtla needs root permission\n"); if (params->common.hist.no_irq && params->common.hist.no_thread) timerlat_hist_usage("no-irq and no-thread set, there is nothing to do here"); diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index fc479a0dcb59..03ecc38d0719 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -671,10 +671,8 @@ static struct common_params break; case 'e': tevent = trace_event_alloc(optarg); - if (!tevent) { - err_msg("Error alloc trace event"); - exit(EXIT_FAILURE); - } + if (!tevent) + fatal("Error alloc trace event"); if (params->common.events) tevent->next = params->common.events; @@ -687,10 +685,8 @@ static struct common_params case 'H': params->common.hk_cpus = 1; retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set); - if (retval) { - err_msg("Error parsing house keeping CPUs\n"); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error parsing house keeping CPUs\n"); break; case 'i': params->common.stop_us = get_llong_from_str(optarg); @@ -741,10 +737,8 @@ static struct common_params case '0': /* trigger */ if (params->common.events) { retval = trace_event_add_trigger(params->common.events, optarg); - if (retval) { - err_msg("Error adding trigger %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding trigger %s\n", optarg); } else { timerlat_top_usage("--trigger requires a previous -e\n"); } @@ -752,20 +746,16 @@ static struct common_params case '1': /* filter */ if (params->common.events) { retval = trace_event_add_filter(params->common.events, optarg); - if (retval) { - err_msg("Error adding filter %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding filter %s\n", optarg); } else { timerlat_top_usage("--filter requires a previous -e\n"); } break; case '2': /* dma-latency */ params->dma_latency = get_llong_from_str(optarg); - if (params->dma_latency < 0 || params->dma_latency > 10000) { - err_msg("--dma-latency needs to be >= 0 and < 10000"); - exit(EXIT_FAILURE); - } + if (params->dma_latency < 0 || params->dma_latency > 10000) + fatal("--dma-latency needs to be >= 0 and < 10000"); break; case '3': /* no-aa */ params->no_aa = 1; @@ -785,18 +775,14 @@ static struct common_params case '9': retval = actions_parse(¶ms->common.threshold_actions, optarg, "timerlat_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s\n", optarg); break; case '\1': retval = actions_parse(¶ms->common.end_actions, optarg, "timerlat_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s\n", optarg); break; default: timerlat_top_usage("Invalid option"); @@ -806,10 +792,8 @@ static struct common_params if (trace_output) actions_add_trace_output(¶ms->common.threshold_actions, trace_output); - if (geteuid()) { - err_msg("rtla needs root permission\n"); - exit(EXIT_FAILURE); - } + if (geteuid()) + fatal("rtla needs root permission\n"); /* * Auto analysis only happens if stop tracing, thus: diff --git a/tools/tracing/rtla/src/timerlat_u.c b/tools/tracing/rtla/src/timerlat_u.c index 01dbf9a6b5a5..2f85c242e1d5 100644 --- a/tools/tracing/rtla/src/timerlat_u.c +++ b/tools/tracing/rtla/src/timerlat_u.c @@ -51,10 +51,8 @@ static int timerlat_u_main(int cpu, struct timerlat_u_params *params) if (!params->sched_param) { retval = sched_setscheduler(0, SCHED_FIFO, &sp); - if (retval < 0) { - err_msg("Error setting timerlat u default priority: %s\n", strerror(errno)); - exit(1); - } + if (retval < 0) + fatal("Error setting timerlat u default priority: %s\n", strerror(errno)); } else { retval = __set_sched_attr(getpid(), params->sched_param); if (retval) { @@ -78,10 +76,8 @@ static int timerlat_u_main(int cpu, struct timerlat_u_params *params) snprintf(buffer, sizeof(buffer), "osnoise/per_cpu/cpu%d/timerlat_fd", cpu); timerlat_fd = tracefs_instance_file_open(NULL, buffer, O_RDONLY); - if (timerlat_fd < 0) { - err_msg("Error opening %s:%s\n", buffer, strerror(errno)); - exit(1); - } + if (timerlat_fd < 0) + fatal("Error opening %s:%s\n", buffer, strerror(errno)); debug_msg("User-space timerlat pid %d on cpu %d\n", gettid(), cpu); diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c index d6ab15dcb490..bf3662313cde 100644 --- a/tools/tracing/rtla/src/utils.c +++ b/tools/tracing/rtla/src/utils.c @@ -38,6 +38,20 @@ void err_msg(const char *fmt, ...) fprintf(stderr, "%s", message); } +/* + * fatal - print an error message to stderr and exit with ERROR + */ +void fatal(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + vfprintf(stderr, fmt, ap); + va_end(ap); + + exit(ERROR); +} + /* * debug_msg - print a debug message to stderr if debug is set */ diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h index a2a6f89f342d..1be095f9a7e6 100644 --- a/tools/tracing/rtla/src/utils.h +++ b/tools/tracing/rtla/src/utils.h @@ -19,6 +19,7 @@ extern int config_debug; void debug_msg(const char *fmt, ...); void err_msg(const char *fmt, ...); +void fatal(const char *fmt, ...); long parse_seconds_duration(char *val); void get_duration(time_t start_time, char *output, int output_size); -- 2.51.0