From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3v4tFf2kyDzDqMd for ; Sat, 21 Jan 2017 07:52:54 +1100 (AEDT) Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v0KKmedf006188 for ; Fri, 20 Jan 2017 15:52:51 -0500 Received: from e32.co.us.ibm.com (e32.co.us.ibm.com [32.97.110.150]) by mx0a-001b2d01.pphosted.com with ESMTP id 282ybhd2da-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 20 Jan 2017 15:52:51 -0500 Received: from localhost by e32.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 20 Jan 2017 13:52:49 -0700 Subject: Re: powerpc/nvram: Move an assignment for the variable "ret" in dev_nvram_write() To: SF Markus Elfring , linuxppc-dev@lists.ozlabs.org References: <53545d97-6ed5-ff17-384f-82e72b3592f9@users.sourceforge.net> <52f46c9b-e049-7288-2c53-9c2525cd84c6@linux.vnet.ibm.com> Cc: Paul Gortmaker , kernel-janitors@vger.kernel.org, LKML , Geliang Tang , Paul Mackerras , Pan Xinhui , Nathan Fontenot , Daniel Axtens From: Tyrel Datwyler Date: Fri, 20 Jan 2017 12:52:44 -0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Message-Id: List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 01/19/2017 11:08 PM, SF Markus Elfring wrote: >> I think you really could have squashed patches 1-3 into a single patch >> that returns directly after any failure. > > Thanks for your constructive feedback. > > I have got software development concerns around such patch squashing. > > >> At this point you might as well remove that label and move the kfree(tmp) call up >> and return directly after the failure and at the nvram_write() call site >> doing away completely with the "ret" variable. > > Your idea might look nice at first glance. But I would interpret the previous > implementation of the discussed function in the way that the memory which was > dynamically allocated here should always (not only in the failure case) be released > before returning here. You are correct. I did muck that part up. However, I do still believe it is cleaner to squash your three patches together. There is no functional change here and it is clearer in a single patch that you are modifying the function to return directly in the simple error cases. -Tyrel > > Would you really like to change the life time for this “temporary” data item? > > Regards, > Markus >