From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751422AbdATHJe (ORCPT <rfc822;w@1wt.eu>);
        Fri, 20 Jan 2017 02:09:34 -0500
Received: from mout.web.de ([212.227.17.12]:53544 "EHLO mout.web.de"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751260AbdATHJd (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 20 Jan 2017 02:09:33 -0500
Subject: Re: powerpc/nvram: Move an assignment for the variable "ret" in
 dev_nvram_write()
To: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>,
        linuxppc-dev@lists.ozlabs.org
References: <53545d97-6ed5-ff17-384f-82e72b3592f9@users.sourceforge.net>
 <dcaccc31-99ac-ef90-cbef-79dbf72f0a4d@users.sourceforge.net>
 <52f46c9b-e049-7288-2c53-9c2525cd84c6@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Daniel Axtens <dja@axtens.net>, Geliang Tang <geliangtang@163.com>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Nathan Fontenot <nfont@linux.vnet.ibm.com>,
        Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>,
        Paul Gortmaker <paul.gortmaker@windriver.com>,
        Paul Mackerras <paulus@samba.org>, kernel-janitors@vger.kernel.org,
        LKML <linux-kernel@vger.kernel.org>
From: SF Markus Elfring <elfring@users.sourceforge.net>
Message-ID: <b0958392-8325-4f56-cded-0ff6691893cf@users.sourceforge.net>
Date: Fri, 20 Jan 2017 08:08:55 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.6.0
MIME-Version: 1.0
In-Reply-To: <52f46c9b-e049-7288-2c53-9c2525cd84c6@linux.vnet.ibm.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
X-Provags-ID: V03:K0:wL1Ete7XiQq4HH1JsdJ0HVQJlHqmAPjCQ1vENa5AKBQRMpyZ/Q+
 PJHxp9bcG6vWUHD2/qoLrOeV5yuG5JEq6c714QwfH7inVUei9CYWYJSbOzAJzqSKC1cMty5
 1diTQIxESEAD9Zy2OWb0NkB/pcaR+XUoVPyF4fsnqJjoIRCzRj3VCoZ5cHrYw/yaeFg/z5u
 K2JiklW7XkruXLdI3Nnig==
X-UI-Out-Filterresults: notjunk:1;V01:K0:JrcBhuYe+DA=:TpjGX39n/t1yRU5pa2sQAa
 w6p2DhkfL1oDx9XMKa0OjZW2v+XKoBwRKYljnyj3SenoWG5YbNgZw4cqr5PIb5WufGWh1fyET
 MvaDOxQslE5DDa1Ujw0b6vO7ENOLpTLXg8w7Km3pA6KbSFdHBtZ81/rHSfqdc64JUKjEsrhWi
 +DDJDCIeB/S/iXcaS+HGpEHEzo3Dg8G1uk2iQxCIWNSmxMcrp5b5vXqecK7PuCvucmFmJmzm0
 wZXPlK3U2GM2J+gDGsMW16tv5Cgq/QAlh3f9eYOwD/gCd67gXv3jukQ7nxjWVgC6Waxk+Wt4Q
 cN2akYGjsJosHvD6n5m7Ib2d3xADGndsZR9GmEnJFOjqaOCxGwKSD6VxZJUT/ET1aYdQdL+Zc
 jVH0uSEbf6HyLNR9vWAx/6+QkUAX21nTDRQD29tCdGePdbFTYtvB7v8Nhwo7quFVPt4EXBhiQ
 tvrROS/Bi4yM5DZWXd5RW/sD4XQV06y66MYmORRv7+Gc6L8vUQjoCWFdWLcSecaIlw8WUibZl
 M2bvAhMmgHpEJz5eL5EKQlsqvyzF3Q5xKijWJnB/B/Cq5XkQJBWHn0MLBMftfwx5+kpOyxkfn
 z9DjUOA1yBlQNYDkOgS2Rd3mGWFNe6FjCZe5ooVWBvS5Dpu53LnY63NroQ8FfcddL8gTL5hbe
 bRD/Dys3jwlxEdkiTH++oJKKW7D45R5FV6Xq2+vB9HncrJhz8Pcrl5cqr2qKWFErXbsxD6frX
 Kq9HzILFECukDFy0840HcZkTxS6Gntjqfe1mCQYIJ1tnzCxZCwB3UQA0zjoS2EIvjvKER86P4
 KVnfKrA
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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

Would you really like to change the life time for this “temporary” data item?

Regards,
Markus