| Submitter | Julia Lawall |
|---|---|
| Date | Aug. 20, 2011, 7:23 a.m. |
| Message ID | <1313825025-17590-1-git-send-email-julia@diku.dk> |
| Download | mbox | patch |
| Permalink | /patch/110760/ |
| State | Not Applicable |
| Headers | show |
Comments
On 20/08/11 08:23, Julia Lawall wrote: > From: Julia Lawall <julia@diku.dk> > > of_parse_phandle increments the reference count of np, so this should be > decremented before trying the next possibility. > > The semantic match that finds this problem is as follows: > (http://coccinelle.lip6.fr/) > > // <smpl> > @@ > expression e,e1,e2; > @@ > > *e = of_parse_phandle(...) > ... when != of_node_put(e) > when != true e == NULL > when != e2 = e > e = e1 > // </smpl> > > Signed-off-by: Julia Lawall <julia@diku.dk> > > --- > sound/soc/fsl/fsl_dma.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c > index 0efc04a..b33271b 100644 > --- a/sound/soc/fsl/fsl_dma.c > +++ b/sound/soc/fsl/fsl_dma.c > @@ -880,10 +880,12 @@ static struct device_node *find_ssi_node(struct device_node *dma_channel_np) > np = of_parse_phandle(ssi_np, "fsl,playback-dma", 0); > if (np == dma_channel_np) > return ssi_np; > + of_node_put(np); > > np = of_parse_phandle(ssi_np, "fsl,capture-dma", 0); > if (np == dma_channel_np) > return ssi_np; > + of_node_put(np); > } > > return NULL; > Acked-by: Liam Girdwood <lrg@ti.com>
Julia Lawall wrote: > diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c > index 0efc04a..b33271b 100644 > --- a/sound/soc/fsl/fsl_dma.c > +++ b/sound/soc/fsl/fsl_dma.c > @@ -880,10 +880,12 @@ static struct device_node *find_ssi_node(struct device_node *dma_channel_np) > np = of_parse_phandle(ssi_np, "fsl,playback-dma", 0); > if (np == dma_channel_np) > return ssi_np; > + of_node_put(np); > > np = of_parse_phandle(ssi_np, "fsl,capture-dma", 0); > if (np == dma_channel_np) > return ssi_np; > + of_node_put(np); > } Thanks for catching the problem, Julia, but the fix is not quite correct. My code assumes that of_parse_phandle() doesn't claim the node, but it doesn't actually use the node pointer, either. All I care about is whether 'np' is equal to dma_channel_np. I'm not going to use 'np'. So I think the real fix is this: @@ -880,10 +880,12 @@ static struct device_node *find_ssi_node(struct device_node *dma_channel_np) np = of_parse_phandle(ssi_np, "fsl,playback-dma", 0); + of_node_put(np); if (np == dma_channel_np) return ssi_np; np = of_parse_phandle(ssi_np, "fsl,capture-dma", 0); + of_node_put(np); if (np == dma_channel_np) return ssi_np; } return NULL;
On Mon, 22 Aug 2011, Timur Tabi wrote: > Julia Lawall wrote: > > diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c > > index 0efc04a..b33271b 100644 > > --- a/sound/soc/fsl/fsl_dma.c > > +++ b/sound/soc/fsl/fsl_dma.c > > @@ -880,10 +880,12 @@ static struct device_node *find_ssi_node(struct device_node *dma_channel_np) > > np = of_parse_phandle(ssi_np, "fsl,playback-dma", 0); > > if (np == dma_channel_np) > > return ssi_np; > > + of_node_put(np); > > > > np = of_parse_phandle(ssi_np, "fsl,capture-dma", 0); > > if (np == dma_channel_np) > > return ssi_np; > > + of_node_put(np); > > } > > Thanks for catching the problem, Julia, but the fix is not quite correct. My > code assumes that of_parse_phandle() doesn't claim the node, but it doesn't > actually use the node pointer, either. All I care about is whether 'np' is > equal to dma_channel_np. I'm not going to use 'np'. So I think the real fix is > this: > > @@ -880,10 +880,12 @@ static struct device_node *find_ssi_node(struct > device_node *dma_channel_np) > np = of_parse_phandle(ssi_np, "fsl,playback-dma", 0); > + of_node_put(np); > if (np == dma_channel_np) > return ssi_np; > > np = of_parse_phandle(ssi_np, "fsl,capture-dma", 0); > + of_node_put(np); > if (np == dma_channel_np) > return ssi_np; > } > > return NULL; OK, that looks reasonable. julia
Patch
diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c index 0efc04a..b33271b 100644 --- a/sound/soc/fsl/fsl_dma.c +++ b/sound/soc/fsl/fsl_dma.c @@ -880,10 +880,12 @@ static struct device_node *find_ssi_node(struct device_node *dma_channel_np) np = of_parse_phandle(ssi_np, "fsl,playback-dma", 0); if (np == dma_channel_np) return ssi_np; + of_node_put(np); np = of_parse_phandle(ssi_np, "fsl,capture-dma", 0); if (np == dma_channel_np) return ssi_np; + of_node_put(np); } return NULL;