Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751857AbdGaTfn (ORCPT ); Mon, 31 Jul 2017 15:35:43 -0400 Received: from mail.kernel.org ([198.145.29.99]:59458 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751708AbdGaTfl (ORCPT ); Mon, 31 Jul 2017 15:35:41 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1358922B4B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=rostedt@goodmis.org Date: Mon, 31 Jul 2017 15:35:39 -0400 From: Steven Rostedt To: Federico Vaga Cc: LKML 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> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1620 Lines: 38 On Mon, 10 Jul 2017 02:15:35 +0200 Federico Vaga 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 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