Re: [PATCH 04/11] trace-cmd: Extract parse_record_options() from trace_record()

From: Vladislav Valtchev <vladislav.valtchev@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, y.karadz@gmail.com
Subject: Re: [PATCH 04/11] trace-cmd: Extract parse_record_options() from trace_record()
Date: Tue, 28 Nov 2017 20:17:46 +0200	[thread overview]
Message-ID: <1511893066.1754.8.camel@gmail.com> (raw)
In-Reply-To: <20171128114822.0f80ed1e@gandalf.local.home>

On Tue, 2017-11-28 at 11:48 -0500, Steven Rostedt wrote:
> 
> Is there a reason that init_instance() isn't called in
> init_common_record_context()?
> 

Hi Steven,

init_instance() has been put into init_common_record_context() later,
in patch 8, "Making start,extract,stream,profile separate funcs".

> Also, why not just do the "init_common_record_context()" in
> parse_record_options()?
> 

Because they do semantically different things: trace_* function might
want to do something before parsing the options, but after initilizing the context.
That is what already happens in trace_profile(): during the refactoring I was able
to remove a heading if (IS_PROFILE(ctx)) containing:

	handle_init = trace_init_profile;
	ctx.events = 1;

and a tail:

	/*
	 * If no instances were set, then enable profiling on the top instance.
	 */
	if (!buffer_instances)
		top_instance.profile = 1;

Since that code was profile-specific, I put it in trace_profile(),
without 'if' statements, clearly.

Vlad