From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751065AbdAQI2L (ORCPT ); Tue, 17 Jan 2017 03:28:11 -0500 Received: from mx07-00178001.pphosted.com ([62.209.51.94]:18971 "EHLO mx07-00178001.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751025AbdAQI2H (ORCPT ); Tue, 17 Jan 2017 03:28:07 -0500 Subject: Re: [patch net-next] stmmac: indent an if statement To: Julia Lawall , Dan Carpenter References: <20170112184631.GA12157@mwanda> <20170115.221438.1956356112755181222.davem@davemloft.net> <20170116091924.GA4104@mwanda> <20170116093954.GB4104@mwanda> <20170116215615.GG4104@mwanda> CC: David Miller , , , , , From: Alexandre Torgue Message-ID: <4902e86a-6a86-57d6-51c2-55e391b83dc3@st.com> Date: Tue, 17 Jan 2017 09:25:21 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.75.127.49] X-ClientProxiedBy: SFHDAG6NODE1.st.com (10.75.127.16) To SFHDAG3NODE2.st.com (10.75.127.8) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-01-17_05:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Julia On 01/16/2017 11:10 PM, Julia Lawall wrote: > > > On Tue, 17 Jan 2017, Dan Carpenter wrote: > >> On Mon, Jan 16, 2017 at 10:46:22PM +0100, Julia Lawall wrote: >>> >>> >>> On Mon, 16 Jan 2017, Dan Carpenter wrote: >>> >>>> On Mon, Jan 16, 2017 at 12:19:24PM +0300, Dan Carpenter wrote: >>>>> On Sun, Jan 15, 2017 at 10:14:38PM -0500, David Miller wrote: >>>>>> From: Dan Carpenter >>>>>> Date: Thu, 12 Jan 2017 21:46:32 +0300 >>>>>> >>>>>>> The break statement should be indented one more tab. >>>>>>> >>>>>>> Signed-off-by: Dan Carpenter >>>>>> >>>>>> Applied, but like Julia I think we might have a missing of_node_put() >>>>>> here. >>>>> >>>>> Of course, sorry for dropping the ball on this. I'll send a patch for >>>>> that. >>>>> >>>> >>>> Actually, I've looked at it some more and I think this function is OK. >>>> We're supposed to do an of_node_put() later... I can't find where that >>>> happens, but presumably that's because I don't know stmmac well. This >>>> code here, though, is fine. >>> >>> Why do you think it is fine? Does anyone in the calling context know >>> which child would have caused the break? >> >> Yeah. It's saved in plat->mdio_node and we expect to be holding on >> either path through the function. >> >> (It would be better if one of the stmmac people were responding here >> insead of a random fix the indenting weenie like myself.) > > OK, I agree that there should not be an of_node_put with the break. > > Perhaps there should be an of_node_put on plat->mdio_node in > stmmac_remove_config_dt, like there is an of_node_put on plat->phy_node. > But it would certainly be helpful to hear from someone who knows the code > better. I also think it's missing! Can you propose a patch ? br Alex > > julia >