Apply PTR_ERR to the value that was recently assigned.
---
var/julia/linuxcopy/drivers/spi/spi-ep93xx.c | 3 ++-
var/julia/linuxcopy/sound/soc/qcom/lpass-cpu.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
Apply PTR_ERR to the value that was recently assigned.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression x,y;
@@
if (IS_ERR(x) || ...) {
... when any
when != IS_ERR(...)
(
PTR_ERR(x)
|
* PTR_ERR(y)
)
... when any
}
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
sound/soc/qcom/lpass-cpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index 23f3d59..94beb99 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -438,7 +438,8 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
if (IS_ERR(drvdata->mi2s_bit_clk[dai_id])) {
dev_err(&pdev->dev,
"%s() error getting mi2s-bit-clk: %ld\n",
- __func__, PTR_ERR(drvdata->mi2s_bit_clk[i]));
+ __func__,
+ PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
}
}
Apply PTR_ERR to the value that was recently assigned.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression x,y;
@@
if (IS_ERR(x) || ...) {
... when any
when != IS_ERR(...)
(
PTR_ERR(x)
|
* PTR_ERR(y)
)
... when any
}
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
drivers/spi/spi-ep93xx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-ep93xx.c b/drivers/spi/spi-ep93xx.c
index bb00be8..1a7696c 100644
--- a/drivers/spi/spi-ep93xx.c
+++ b/drivers/spi/spi-ep93xx.c
@@ -567,7 +567,8 @@ static void ep93xx_spi_dma_transfer(struct ep93xx_spi *espi)
txd = ep93xx_spi_dma_prepare(espi, DMA_MEM_TO_DEV);
if (IS_ERR(txd)) {
ep93xx_spi_dma_finish(espi, DMA_DEV_TO_MEM);
- dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n", PTR_ERR(rxd));
+ dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n",
+ PTR_ERR(txd));
msg->status = PTR_ERR(txd);
return;
}
Am 30.08.2015 20:05, schrieb Julia Lawall:
> Apply PTR_ERR to the value that was recently assigned.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression x,y;
> @@
>
> if (IS_ERR(x) || ...) {
> ... when any
> when != IS_ERR(...)
> (
> PTR_ERR(x)
> |
> * PTR_ERR(y)
> )
> ... when any
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
>
> ---
> drivers/spi/spi-ep93xx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-ep93xx.c b/drivers/spi/spi-ep93xx.c
> index bb00be8..1a7696c 100644
> --- a/drivers/spi/spi-ep93xx.c
> +++ b/drivers/spi/spi-ep93xx.c
> @@ -567,7 +567,8 @@ static void ep93xx_spi_dma_transfer(struct ep93xx_spi *espi)
> txd = ep93xx_spi_dma_prepare(espi, DMA_MEM_TO_DEV);
> if (IS_ERR(txd)) {
> ep93xx_spi_dma_finish(espi, DMA_DEV_TO_MEM);
> - dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n", PTR_ERR(rxd));
> + dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n",
> + PTR_ERR(txd));
> msg->status = PTR_ERR(txd);
> return;
> }
>
I improve readability i would suggest:
ep93xx_spi_dma_finish(espi, DMA_DEV_TO_MEM);
msg->status = PTR_ERR(txd);
dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n",msg->status):
return;
just my 2 cents,
re,
wh
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Am 30.08.2015 20:05, schrieb Julia Lawall:
> Apply PTR_ERR to the value that was recently assigned.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression x,y;
> @@
>
> if (IS_ERR(x) || ...) {
> ... when any
> when != IS_ERR(...)
> (
> PTR_ERR(x)
> |
> * PTR_ERR(y)
> )
> ... when any
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
>
> ---
> sound/soc/qcom/lpass-cpu.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
> index 23f3d59..94beb99 100644
> --- a/sound/soc/qcom/lpass-cpu.c
> +++ b/sound/soc/qcom/lpass-cpu.c
> @@ -438,7 +438,8 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
> if (IS_ERR(drvdata->mi2s_bit_clk[dai_id])) {
> dev_err(&pdev->dev,
> "%s() error getting mi2s-bit-clk: %ld\n",
> - __func__, PTR_ERR(drvdata->mi2s_bit_clk[i]));
> + __func__,
> + PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
> return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
> }
> }
>
just a note:
using a shorter name instead of drvdata->mi2s_bit_clk[dai_id] whould help to make the code
more readable (yes, the other code is alike). something like:
struct clk *tmp = devm_clk_get(&pdev->dev,clk_name);
if (IS_ERR(tmp)) {
dev_err(&pdev->dev,"%s() error getting mi2s-bit-clk: %ld\n",__func__, PTR_ERR(tmp));
return PTR_ERR(tmp);
}
drvdata->mi2s_bit_clk[dai_id]=tmp;
just one minor:
the dev_warn() just before says: " error getting mi2s-osr-clk" may be it should be "warnig ..."
That will make it more easy to rep for real error in a log.
re,
wh
On Sun, 30 Aug 2015, walter harms wrote:
>
>
> Am 30.08.2015 20:05, schrieb Julia Lawall:
> > Apply PTR_ERR to the value that was recently assigned.
> >
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression x,y;
> > @@
> >
> > if (IS_ERR(x) || ...) {
> > ... when any
> > when != IS_ERR(...)
> > (
> > PTR_ERR(x)
> > |
> > * PTR_ERR(y)
> > )
> > ... when any
> > }
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <[email protected]>
> >
> > ---
> > sound/soc/qcom/lpass-cpu.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
> > index 23f3d59..94beb99 100644
> > --- a/sound/soc/qcom/lpass-cpu.c
> > +++ b/sound/soc/qcom/lpass-cpu.c
> > @@ -438,7 +438,8 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
> > if (IS_ERR(drvdata->mi2s_bit_clk[dai_id])) {
> > dev_err(&pdev->dev,
> > "%s() error getting mi2s-bit-clk: %ld\n",
> > - __func__, PTR_ERR(drvdata->mi2s_bit_clk[i]));
> > + __func__,
> > + PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
> > return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
> > }
> > }
> >
>
> just a note:
> using a shorter name instead of drvdata->mi2s_bit_clk[dai_id] whould help to make the code
> more readable (yes, the other code is alike). something like:
>
> struct clk *tmp = devm_clk_get(&pdev->dev,clk_name);
Where do you suggest to put this?
Maybe it would be reasonable to declare a variable struct clk *clk; at the
top of the function, and then use that as a temporary variable for all
three calls.
However, now I see that the first call, unlike the other two doesn't cause
a return from the function.
if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) {
dev_warn(&pdev->dev,
"%s() error getting mi2s-osr-clk: %ld\n",
__func__,
PTR_ERR(drvdata->mi2s_osr_clk[dai_id]));
}
Is that intentional?
thanks,
julia
> if (IS_ERR(tmp)) {
> dev_err(&pdev->dev,"%s() error getting mi2s-bit-clk: %ld\n",__func__, PTR_ERR(tmp));
> return PTR_ERR(tmp);
> }
> drvdata->mi2s_bit_clk[dai_id]=tmp;
>
>
> just one minor:
> the dev_warn() just before says: " error getting mi2s-osr-clk" may be it should be "warnig ..."
> That will make it more easy to rep for real error in a log.
>
> re,
> wh
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Move initialization of msg->status up over the call to dev_err, in both
calls to ep93xx_spi_dma_prepare, and change the reference in the call to
dev_err to msg->status, to both fix the wrong argument to PTR_ERR in the
second case and to make the dev_err line a little shorter. This required
furthermore replacing %ld by %d, since msg->status is an integer.
The semantic match that finds the PTR_ERR problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression x,y;
@@
if (IS_ERR(x) || ...) {
... when any
when != IS_ERR(...)
(
PTR_ERR(x)
|
* PTR_ERR(y)
)
... when any
}
// </smpl>
Reorganizations at the suggestion of Walter Harms.
Signed-off-by: Julia Lawall <[email protected]>
---
v2: Reorganize the code, to solve the problem in a way that makes the
resulting code simpler.
drivers/spi/spi-ep93xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-ep93xx.c b/drivers/spi/spi-ep93xx.c
index bb00be8..73d0df6 100644
--- a/drivers/spi/spi-ep93xx.c
+++ b/drivers/spi/spi-ep93xx.c
@@ -559,16 +559,16 @@ static void ep93xx_spi_dma_transfer(struct ep93xx_spi *espi)
rxd = ep93xx_spi_dma_prepare(espi, DMA_DEV_TO_MEM);
if (IS_ERR(rxd)) {
- dev_err(&espi->pdev->dev, "DMA RX failed: %ld\n", PTR_ERR(rxd));
msg->status = PTR_ERR(rxd);
+ dev_err(&espi->pdev->dev, "DMA RX failed: %d\n", msg->status);
return;
}
txd = ep93xx_spi_dma_prepare(espi, DMA_MEM_TO_DEV);
if (IS_ERR(txd)) {
ep93xx_spi_dma_finish(espi, DMA_DEV_TO_MEM);
- dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n", PTR_ERR(rxd));
msg->status = PTR_ERR(txd);
+ dev_err(&espi->pdev->dev, "DMA TX failed: %d\n", msg->status);
return;
}