Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753154AbdGJAPk (ORCPT ); Sun, 9 Jul 2017 20:15:40 -0400 Received: from mx.kolabnow.com ([95.128.36.42]:49394 "EHLO mx.kolabnow.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753110AbdGJAPj (ORCPT ); Sun, 9 Jul 2017 20:15:39 -0400 From: Federico Vaga To: Steven Rostedt Cc: LKML Subject: Re: [PATCH 2/2] trace-cmd: replace show_file() -> show_instance_file() Date: Mon, 10 Jul 2017 02:15:35 +0200 Message-ID: <15469211.xD5oAuMFTH@harkonnen> Organization: VAGA In-Reply-To: <20170706182935.7a72ed14@gandalf.local.home> References: <20170605093118.29962-1-federico.vaga@vaga.pv.it> <20170605093118.29962-3-federico.vaga@vaga.pv.it> <20170706182935.7a72ed14@gandalf.local.home> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1674 Lines: 47 On Friday, July 7, 2017 12:29:35 AM CEST Steven Rostedt wrote: > On Mon, 5 Jun 2017 11:31:18 +0200 > > Federico Vaga 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 > > --- > > > > 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/