DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1358922B4B
Date: Mon, 31 Jul 2017 15:35:39 -0400
From: Steven Rostedt <rostedt@goodmis.org>
To: Federico Vaga <federico.vaga@vaga.pv.it>
Cc: LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] trace-cmd: replace show_file() ->
 show_instance_file()
Message-ID: <20170731153539.255ab561@gandalf.local.home>
In-Reply-To: <15469211.xD5oAuMFTH@harkonnen>
References: <20170605093118.29962-1-federico.vaga@vaga.pv.it>
        <20170605093118.29962-3-federico.vaga@vaga.pv.it>
        <20170706182935.7a72ed14@gandalf.local.home>
        <15469211.xD5oAuMFTH@harkonnen>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1620
Lines: 38

On Mon, 10 Jul 2017 02:15:35 +0200
Federico Vaga <federico.vaga@vaga.pv.it> wrote:

> 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.
> 

Just an FYI. You'll find lots of these types of helper functions in the
Linux Kernel. As I'm a Linux Kernel developer, I prefer them ;-)

-- Steve