Re: [patch net-next] stmmac: indent an if statement

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <dan.carpenter@xxxxxxxxxx>
Date: Thu, 12 Jan 2017 21:46:32 +0300

The break statement should be indented one more tab.

Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>

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

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux