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

[Vladislav Valtchev]

On Tue, 2017-11-28 at 13:30 -0500, Steven Rostedt wrote:
> On Tue, 28 Nov 2017 20:17:46 +0200
> Vladislav Valtchev <vladislav.valtchev@xxxxxxxxx> wrote:
> 
> > Since that code was profile-specific, I put it in trace_profile(),
> > without 'if' statements, clearly.
> 
> This is all about balancing. You were able to remove one if statement,
> but required two function calls by all others.
> 

I totally agree that is all about balancing.
I wrote exactly the same thing as part of my previous e-mail (before reading this message).

Are you concerned by the cost the of function calls or by the "verbosity" in calling them?

If it's about their cost, I believe that cold code paths like these, where the program is
just parsing its command line arguments, it seems absolutely OK to me to do that.
It's a one-time thing before any kind of tracing starts. And it helps a lot contributors to
better understand the code (in my opinion).

Otherwise, I believe that having those function calls that way just makes the code simpler
for new contributors. Functions, among everything else, are also "labels" for pieces of code.
Having two different labels there (init context and parse options) makes sense to me.
But, again, I understand that is a trade-off based on my subjective value system and
way of thinking, of course.

Do you have a strong opinion?

Vlad