[PATCH 06/11] trace-cmd: Replacing cmd flags w/ a trace_cmd enum

From: Vladislav Valtchev (VMware)
Date: Thu Nov 23 2017 - 11:35:03 EST


This patch replaces 5 mutually exclusive flag variables (extract, start, stream,
record, profile) in trace_record() with a more convenient enum. The point of
doing that is to make the code simpler and easier to maintain.

Signed-off-by: Vladislav Valtchev (VMware) <vladislav.valtchev@xxxxxxxxx>
---
trace-record.c | 94 ++++++++++++++++++++++++++++++++--------------------------
1 file changed, 52 insertions(+), 42 deletions(-)

diff --git a/trace-record.c b/trace-record.c
index b75d209..2d74a68 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -4358,7 +4358,16 @@ void trace_reset(int argc, char **argv)
exit(0);
}

+enum trace_cmd {
+ CMD_extract,
+ CMD_start,
+ CMD_stream,
+ CMD_profile,
+ CMD_record
+};
+
struct common_record_context {
+ enum trace_cmd curr_cmd;
struct buffer_instance *instance;
const char *output;
char *date2ts;
@@ -4376,14 +4385,14 @@ struct common_record_context {
int topt;
int do_child;
int run_command;
-
- int extract;
- int start;
- int stream;
- int profile;
- int record;
};

+#define IS_EXTRACT(ctx) ((ctx)->curr_cmd == CMD_extract)
+#define IS_START(ctx) ((ctx)->curr_cmd == CMD_start)
+#define IS_STREAM(ctx) ((ctx)->curr_cmd == CMD_stream)
+#define IS_PROFILE(ctx) ((ctx)->curr_cmd == CMD_profile)
+#define IS_RECORD(ctx) ((ctx)->curr_cmd == CMD_record)
+

static void parse_record_options(int argc,
char **argv,
@@ -4419,7 +4428,7 @@ static void parse_record_options(int argc,
{NULL, 0, NULL, 0}
};

- if (ctx->extract)
+ if (IS_EXTRACT(ctx))
opts = "+haf:Fp:co:O:sr:g:l:n:P:N:tb:B:ksiT";
else
opts = "+hae:f:Fp:cC:dDGo:O:s:r:vg:l:n:P:N:tb:R:B:ksSiTm:M:H:q";
@@ -4431,7 +4440,7 @@ static void parse_record_options(int argc,
usage(argv);
break;
case 'a':
- if (ctx->extract) {
+ if (IS_EXTRACT(ctx)) {
add_all_instances();
} else {
ctx->record_all = 1;
@@ -4550,17 +4559,17 @@ static void parse_record_options(int argc,
case 'o':
if (host)
die("-o incompatible with -N");
- if (ctx->start)
+ if (IS_START(ctx))
die("start does not take output\n"
"Did you mean 'record'?");
- if (ctx->stream)
+ if (IS_STREAM(ctx))
die("stream does not take output\n"
"Did you mean 'record'?");
if (ctx->output)
die("only one output file allowed");
ctx->output = optarg;

- if (ctx->profile) {
+ if (IS_PROFILE(ctx)) {
int fd;

/* pipe the output to this file instead of stdout */
@@ -4587,7 +4596,7 @@ static void parse_record_options(int argc,
ctx->events = 1;
break;
case 's':
- if (ctx->extract) {
+ if (IS_EXTRACT(ctx)) {
if (optarg)
usage(argv);
recorder_flags |= TRACECMD_RECORD_SNAPSHOT;
@@ -4607,7 +4616,7 @@ static void parse_record_options(int argc,
rt_prio = atoi(optarg);
break;
case 'N':
- if (!ctx->record)
+ if (!IS_RECORD(ctx))
die("-N only available with record");
if (ctx->output)
die("-N incompatible with -o");
@@ -4616,7 +4625,7 @@ static void parse_record_options(int argc,
case 'm':
if (max_kb)
die("-m can only be specified once");
- if (!ctx->record)
+ if (!IS_RECORD(ctx))
die("only record take 'm' option");
max_kb = atoi(optarg);
break;
@@ -4624,7 +4633,7 @@ static void parse_record_options(int argc,
ctx->instance->cpumask = alloc_mask_from_hex(optarg);
break;
case 't':
- if (ctx->extract)
+ if (IS_EXTRACT(ctx))
ctx->topt = 1; /* Extract top instance also */
else
use_tcp = 1;
@@ -4637,7 +4646,7 @@ static void parse_record_options(int argc,
if (!ctx->instance)
die("Failed to create instance");
add_instance(ctx->instance);
- if (ctx->profile)
+ if (IS_PROFILE(ctx))
ctx->instance->profile = 1;
break;
case 'k':
@@ -4715,10 +4724,10 @@ static void parse_record_options(int argc,
die(" -c can only be used with -P or -F");

if ((argc - optind) >= 2) {
- if (ctx->start)
+ if (IS_START(ctx))
die("Command start does not take any commands\n"
"Did you mean 'record'?");
- if (ctx->extract)
+ if (IS_EXTRACT(ctx))
die("Command extract does not take any commands\n"
"Did you mean 'record'?");
ctx->run_command = 1;
@@ -4741,15 +4750,16 @@ void trace_record(int argc, char **argv)
init_common_record_context(&ctx);
init_instance(ctx.instance);

- if ((ctx.record = (strcmp(argv[1], "record") == 0)))
- ; /* do nothing */
- else if ((ctx.start = strcmp(argv[1], "start") == 0))
- ; /* do nothing */
- else if ((ctx.extract = strcmp(argv[1], "extract") == 0))
- ; /* do nothing */
- else if ((ctx.stream = strcmp(argv[1], "stream") == 0))
- ; /* do nothing */
- else if ((ctx.profile = strcmp(argv[1], "profile") == 0)) {
+ if (strcmp(argv[1], "record") == 0)
+ ctx.curr_cmd = CMD_record;
+ else if (strcmp(argv[1], "start") == 0)
+ ctx.curr_cmd = CMD_start;
+ else if (strcmp(argv[1], "extract") == 0)
+ ctx.curr_cmd = CMD_extract;
+ else if (strcmp(argv[1], "stream") == 0)
+ ctx.curr_cmd = CMD_stream;
+ else if (strcmp(argv[1], "profile") == 0) {
+ ctx.curr_cmd = CMD_profile;
handle_init = trace_init_profile;
ctx.events = 1;
} else
@@ -4763,14 +4773,14 @@ void trace_record(int argc, char **argv)
* If this is a profile run, and no instances were set,
* then enable profiling on the top instance.
*/
- if (ctx.profile && !buffer_instances)
+ if (IS_PROFILE(&ctx) && !buffer_instances)
top_instance.profile = 1;

/*
* If top_instance doesn't have any plugins or events, then
* remove it from being processed.
*/
- if (!ctx.extract && !__check_doing_something(&top_instance)) {
+ if (!IS_EXTRACT(&ctx) && !__check_doing_something(&top_instance)) {
if (!buffer_instances)
die("No instances reference??");
first_instance = buffer_instances;
@@ -4779,7 +4789,7 @@ void trace_record(int argc, char **argv)

update_first_instance(ctx.instance, ctx.topt);

- if (!ctx.extract)
+ if (!IS_EXTRACT(&ctx))
check_doing_something();
check_function_plugin();

@@ -4799,10 +4809,10 @@ void trace_record(int argc, char **argv)
}

/* Extracting data records all events in the system. */
- if (ctx.extract && !ctx.record_all)
+ if (IS_EXTRACT(&ctx) && !ctx.record_all)
record_all_events();

- if (!ctx.extract)
+ if (!IS_EXTRACT(&ctx))
make_instances();

if (ctx.events)
@@ -4810,7 +4820,7 @@ void trace_record(int argc, char **argv)

page_size = getpagesize();

- if (!ctx.extract) {
+ if (!IS_EXTRACT(&ctx)) {
fset = set_ftrace(!ctx.disable, ctx.total_disable);
tracecmd_disable_all_tracing(1);

@@ -4818,7 +4828,7 @@ void trace_record(int argc, char **argv)
set_clock(ctx.instance);

/* Record records the date first */
- if (ctx.record && ctx.date)
+ if (IS_RECORD(&ctx) && ctx.date)
ctx.date2ts = get_date_to_ts();

for_all_instances(ctx.instance) {
@@ -4833,13 +4843,13 @@ void trace_record(int argc, char **argv)
set_buffer_size();
}

- if (ctx.record)
+ if (IS_RECORD(&ctx))
type = TRACE_TYPE_RECORD;
- else if (ctx.stream)
+ else if (IS_STREAM(&ctx))
type = TRACE_TYPE_STREAM;
- else if (ctx.extract)
+ else if (IS_EXTRACT(&ctx))
type = TRACE_TYPE_EXTRACT;
- else if (ctx.profile)
+ else if (IS_PROFILE(&ctx))
type = TRACE_TYPE_STREAM;
else
type = TRACE_TYPE_START;
@@ -4862,7 +4872,7 @@ void trace_record(int argc, char **argv)
start_threads(type, ctx.global);
}

- if (ctx.extract) {
+ if (IS_EXTRACT(&ctx)) {
flush_threads();

} else {
@@ -4897,7 +4907,7 @@ void trace_record(int argc, char **argv)
tracecmd_disable_all_tracing(0);

/* extract records the date after extraction */
- if (ctx.extract && ctx.date) {
+ if (IS_EXTRACT(&ctx) && ctx.date) {
/*
* We need to start tracing, don't let other traces
* screw with our trace_marker.
@@ -4906,7 +4916,7 @@ void trace_record(int argc, char **argv)
ctx.date2ts = get_date_to_ts();
}

- if (ctx.record || ctx.extract) {
+ if (IS_RECORD(&ctx) || IS_EXTRACT(&ctx)) {
record_data(ctx.date2ts, ctx.data_flags);
delete_thread_data();
} else
@@ -4936,7 +4946,7 @@ void trace_record(int argc, char **argv)
if (host)
tracecmd_output_close(network_handle);

- if (ctx.profile)
+ if (IS_PROFILE(&ctx))
trace_profile_int();

exit(0);
--
2.14.1