From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from k22.active24.pl ([195.78.67.22]:46777 "EHLO k22.active24.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932164AbcFBJRA (ORCPT ); Thu, 2 Jun 2016 05:17:00 -0400 Subject: Re: freevxfs: hp-ux support. patchset 1-7/7 rev 2 From: Krzysztof =?UTF-8?Q?B=C5=82aszkowski?= To: Christoph Hellwig Cc: Carlos Maiolino , linux-fsdevel@vger.kernel.org In-Reply-To: <20160602082518.GB12071@infradead.org> References: <1464273946.17980.15.camel@linux-q3cb.site> <1464464428.3689.14.camel@linux-q3cb.site> <20160531122510.GA25651@infradead.org> <1464702291.900.75.camel@linux-q3cb.site> <20160601073310.GA6787@infradead.org> <1464773012.900.105.camel@linux-q3cb.site> <20160602082518.GB12071@infradead.org> Content-Type: text/plain; charset="UTF-8" Date: Thu, 02 Jun 2016 11:16:58 +0200 Message-ID: <1464859018.4321.7.camel@linux-q3cb.site> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, 2016-06-02 at 01:25 -0700, Christoph Hellwig wrote: > On Wed, Jun 01, 2016 at 11:23:32AM +0200, Krzysztof B??aszkowski wrote: > > I think that there are no "kfree(pfp); kfree(sfp); return 0;" in > > vxfs_read_fshead() still. Are pfp and sfp needed anywhere ? I am sure > > they are not so there is a memory leak without these kfrees every mount. > > > > I am not sure absolutely of that read_fshead() is missing these kfrees > > because I have seen just these diffs, anyway I did not notice "+kfree". > > The frees are still missing. Do you want to send me a patch for those? I see. Don't hesitate to add them, I was thinking that creating a special patch because of these just two kfree()s is just too much formal work. > > > needles to say that I prefer to have limited scope of visibility of > > inode_cachep to the inode.c only. > > In my tree the visibility is in vxfs_super.c only. Given that the > alloc/destroy methods are super operations that seems to fit better > as they can have local scope, too. I see. We differ in point of views. I presumed that it is more consistent to have everything regarding inodes creation (getting)/destroying, etc in the inode.c. Anyway this is just pure academic debate, we could argue on anything else (weather, politics and politicians - hot topic especially in Poland) as well. You decide. You enjoy more your approach - good enough. -- Krzysztof Blaszkowski