lkml.org 
[lkml]   [2017]   [Jul]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 2/2] trace-cmd: replace show_file() -> show_instance_file()
Date
On Friday, July 7, 2017 12:29:35 AM CEST Steven Rostedt wrote:
> On Mon, 5 Jun 2017 11:31:18 +0200
>
> Federico Vaga <federico.vaga@vaga.pv.it> wrote:
> > show_file(name) and show_instance_file(&top_instance, name) are
> > equivalent.
> >
> > Remove the show_file() function in order to have a single function for
> > this task.
>
> Actually I find nothing wrong with having a helper function like this.
> IIRC, show_file() was first, and then show_instance_file() came later.
> There's some files that only exist for the top_instance, and I like the
> fact that this is annotated that way.
>
> I'm curious to know what the benefit of removing show_file() is?

The show_file(name) and show_instance_file(&top_instance, name) are
equivalent: they do the same thing. By removing `show_file` the developers are
forced to use always the same function and being explicit about the instance
they want to use.

The name `show_file()` is so generic that does not implies automatically that
we are accessing the top_instance. This is not even clear by reading the
implementation; people must read the other functions used in `show_file()` to
understand that their instance scope is always 'top_instance'.

So, in my opinion, it makes the code easier to read and more explicit in what
is doing without too much effort.

> -- Steve
>
> > Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
> > ---
> >
> > trace-list.c | 21 ++++++---------------
> > trace-local.h | 2 --
> > trace-show.c | 18 ++----------------
> > trace-snapshot.c | 2 +-
> > 4 files changed, 9 insertions(+), 34 deletions(-)
>
> \


--
Federico Vaga
http://www.federicovaga.it/

\
 
 \ /
  Last update: 2017-07-10 02:16    [W:0.073 / U:0.356 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site