From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753242AbdK1ROx (ORCPT ); Tue, 28 Nov 2017 12:14:53 -0500 Received: from mail.kernel.org ([198.145.29.99]:38042 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751689AbdK1ROw (ORCPT ); Tue, 28 Nov 2017 12:14:52 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B5F25218D9 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=rostedt@goodmis.org Date: Tue, 28 Nov 2017 12:14:50 -0500 From: Steven Rostedt To: "Vladislav Valtchev (VMware)" Cc: linux-kernel@vger.kernel.org, y.karadz@gmail.com Subject: Re: [PATCH 08/11] trace-cmd: Making start,extract,stream,profile separate funcs Message-ID: <20171128121450.32db5781@gandalf.local.home> In-Reply-To: <20171123163335.19078-9-vladislav.valtchev@gmail.com> References: <20171123163335.19078-1-vladislav.valtchev@gmail.com> <20171123163335.19078-9-vladislav.valtchev@gmail.com> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 23 Nov 2017 18:33:32 +0200 "Vladislav Valtchev (VMware)" wrote: > +static void init_common_record_context(struct common_record_context *ctx, > + enum trace_cmd curr_cmd) > +{ > + memset(ctx, 0, sizeof(*ctx)); > + ctx->instance = &top_instance; > + ctx->curr_cmd = curr_cmd; > + init_instance(ctx->instance); > + cpu_count = count_cpus(); > +} > > +void trace_start(int argc, char **argv) > +{ > + struct common_record_context ctx; > + > + init_common_record_context(&ctx, CMD_start); > + parse_record_options(argc, argv, &ctx); > + record_trace(argc, argv, &ctx); > + exit(0); > +} > + > +void trace_extract(int argc, char **argv) > +{ > + struct common_record_context ctx; > + > + init_common_record_context(&ctx, CMD_extract); > + parse_record_options(argc, argv, &ctx); > + record_trace(argc, argv, &ctx); > + exit(0); > +} > + > +void trace_stream(int argc, char **argv) > +{ > + struct common_record_context ctx; > + > + init_common_record_context(&ctx, CMD_stream); > + parse_record_options(argc, argv, &ctx); > + record_trace(argc, argv, &ctx); > + exit(0); > +} > + > +void trace_profile(int argc, char **argv) > +{ > + struct common_record_context ctx; > + > + init_common_record_context(&ctx, CMD_profile); > + > + handle_init = trace_init_profile; > + ctx.events = 1; Why setting events = 1? > + > + parse_record_options(argc, argv, &ctx); > + > + /* > + * If no instances were set, then enable profiling on the top instance. > + */ > + if (!buffer_instances) > + top_instance.profile = 1; > + > + record_trace(argc, argv, &ctx); > + trace_profile_int(); > exit(0); > } > > @@ -4934,24 +4980,8 @@ void trace_record(int argc, char **argv) > { > struct common_record_context ctx; > > - init_common_record_context(&ctx); > - init_instance(ctx.instance); > - > - 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 > - usage(argv); > - > + init_common_record_context(&ctx, CMD_record); > parse_record_options(argc, argv, &ctx); As everything is doing both init_common_record_context() and parse_record_options() why not just move the init_common_record_context() into parse_record_options(). If you need to do something special (like set events = 1 for profile) have that done in parse_record_options() if the cmd is CMD_record. Yes, you need to pass the CMD_* to parse_record_options() in this case. -- Steve > record_trace(argc, argv, &ctx); > + exit(0); > }