Re: [alsa-devel] [PATCH v3] ASoC: Remove 'const' from the device_node pointers
On Tue, Nov 25, 2014 at 12:14:48PM +0100, Jean-Francois Moine wrote:
As Russell King's explained it, there should not be pointers to struct device_node:
"struct device_node is a ref-counted structure. That means if you store a reference to it, you should "get" it, and you should "put" it once you've done. The act of "put"ing the pointed-to structure involves writing to that structure, so it is totally unappropriate to store a device_node structure as a const pointer. It forces you to have to cast it back to a non-const pointer at various points in time to use various OF function calls."
So, we're not holding references here (we're just doing comparisons, the references need to be owned before we get into the core) and I'm not seeing anything here removing casts?
On 11/25/2014 02:13 PM, Mark Brown wrote:
On Tue, Nov 25, 2014 at 12:14:48PM +0100, Jean-Francois Moine wrote:
As Russell King's explained it, there should not be pointers to struct device_node:
"struct device_node is a ref-counted structure. That means if you store a reference to it, you should "get" it, and you should "put" it once you've done. The act of "put"ing the pointed-to structure involves writing to that structure, so it is totally unappropriate to store a device_node structure as a const pointer. It forces you to have to cast it back to a non-const pointer at various points in time to use various OF function calls."
So, we're not holding references here (we're just doing comparisons, the references need to be owned before we get into the core)
The core itself will only do the comparisons and it is the board drivers responsibility to get and put the references. Making the pointers non const allows the board driver to use them to put the reference once the card has been unregistered rather than having to keep a separate set of pointers around. This should probably be mentioned in the commit message though.
I'm not seeing anything here removing casts?
This patch used to be part of a two part series where the second patch removed the casts. This patch as already been applied though.
On Tue, Nov 25, 2014 at 02:36:15PM +0100, Lars-Peter Clausen wrote:
On 11/25/2014 02:13 PM, Mark Brown wrote:
So, we're not holding references here (we're just doing comparisons, the references need to be owned before we get into the core)
The core itself will only do the comparisons and it is the board drivers responsibility to get and put the references. Making the pointers non const allows the board driver to use them to put the reference once the card has been unregistered rather than having to keep a separate set of pointers
Wouldn't it be even better to have managed OF references and not need to explicitly dereference at all? Otherwise every time something uses managed resources for the card or component we've got a (marginal admittedly) reference management bug.
around. This should probably be mentioned in the commit message though.
Yes, it really should - this is really what I'm getting at here since I'm frequently having to push back on difficult to understand changes here.
I'm not seeing anything here removing casts?
This patch used to be part of a two part series where the second patch removed the casts. This patch as already been applied though.
We don't have warnings at the minute...
participants (2)
-
Lars-Peter Clausen -
Mark Brown