[PATCH 1/2] media: pci: saa7164: remove unnecessary code

[Gustavo A. R. Silva]

Remove unnecessary variable 'loop'.

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/pci/saa7164/saa7164-cmd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/pci/saa7164/saa7164-cmd.c 
b/drivers/media/pci/saa7164/saa7164-cmd.c
index 45951b3..169c90a 100644
--- a/drivers/media/pci/saa7164/saa7164-cmd.c
+++ b/drivers/media/pci/saa7164/saa7164-cmd.c
@@ -134,14 +134,13 @@ int saa7164_irq_dequeue(struct saa7164_dev *dev)
  * -bus/c running buffer. */
 static int saa7164_cmd_dequeue(struct saa7164_dev *dev)
 {
-   int loop = 1;
int ret;
u32 timeout;
wait_queue_head_t *q = NULL;
u8 tmp[512];
dprintk(DBGLVL_CMD, "%s()\n", __func__);
 
-   while (loop) {
+   while (true) {
 
struct tmComResInfo tRsp = { 0, 0, 0, 0, 0, 0 };
ret = saa7164_bus_get(dev, , NULL, 1);
-- 
2.5.0

[PATCH 2/2] media: pci: saa7164: remove dead code

[Gustavo A. R. Silva]

Remove dead code. The following line of code is never reached:
return SAA_OK;

Addresses-Coverity-ID: 114283
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/pci/saa7164/saa7164-cmd.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/pci/saa7164/saa7164-cmd.c 
b/drivers/media/pci/saa7164/saa7164-cmd.c
index 169c90a..fb19498 100644
--- a/drivers/media/pci/saa7164/saa7164-cmd.c
+++ b/drivers/media/pci/saa7164/saa7164-cmd.c
@@ -181,8 +181,6 @@ static int saa7164_cmd_dequeue(struct saa7164_dev *dev)
wake_up(q);
return SAA_OK;
}
-
-   return SAA_OK;
 }
 
 static int saa7164_cmd_set(struct saa7164_dev *dev, struct tmComResInfo *msg,
-- 
2.5.0

[PATCH] au0828: fix unbalanced lock/unlock in au0828_usb_probe

[Gustavo A. R. Silva]

Call mutex_unlock and free dev on failure.

Reported-by: Julia Lawall <julia.law...@lip6.fr>
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/usb/au0828/au0828-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/usb/au0828/au0828-core.c 
b/drivers/media/usb/au0828/au0828-core.c
index 739df61..cd363a2 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -628,6 +628,8 @@ static int au0828_usb_probe(struct usb_interface *interface,
if (retval) {
pr_err("%s() au0282_dev_register failed to register on V4L2\n",
__func__);
+   mutex_unlock(>lock);
+   kfree(dev);
goto done;
}
 
-- 
2.5.0

[PATCH] marvell-ccic: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/marvell-ccic/cafe-driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c 
b/drivers/media/platform/marvell-ccic/cafe-driver.c
index 77890bd..063fd43 100644
--- a/drivers/media/platform/marvell-ccic/cafe-driver.c
+++ b/drivers/media/platform/marvell-ccic/cafe-driver.c
@@ -326,7 +326,7 @@ static u32 cafe_smbus_func(struct i2c_adapter *adapter)
   I2C_FUNC_SMBUS_WRITE_BYTE_DATA;
 }
 
-static struct i2c_algorithm cafe_smbus_algo = {
+static const struct i2c_algorithm cafe_smbus_algo = {
.smbus_xfer = cafe_smbus_xfer,
.functionality = cafe_smbus_func
 };
-- 
2.5.0

[PATCH] s5h1420: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/dvb-frontends/s5h1420.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/s5h1420.c 
b/drivers/media/dvb-frontends/s5h1420.c
index cba9bff..fd427a29 100644
--- a/drivers/media/dvb-frontends/s5h1420.c
+++ b/drivers/media/dvb-frontends/s5h1420.c
@@ -864,7 +864,7 @@ static int s5h1420_tuner_i2c_tuner_xfer(struct i2c_adapter 
*i2c_adap, struct i2c
return i2c_transfer(state->i2c, m, 1 + num) == 1 + num ? num : -EIO;
 }
 
-static struct i2c_algorithm s5h1420_tuner_i2c_algo = {
+static const struct i2c_algorithm s5h1420_tuner_i2c_algo = {
.master_xfer   = s5h1420_tuner_i2c_tuner_xfer,
.functionality = s5h1420_tuner_i2c_func,
 };
-- 
2.5.0

[PATCH] saa7146: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/common/saa7146/saa7146_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/common/saa7146/saa7146_i2c.c 
b/drivers/media/common/saa7146/saa7146_i2c.c
index 239a2db..75897f9 100644
--- a/drivers/media/common/saa7146/saa7146_i2c.c
+++ b/drivers/media/common/saa7146/saa7146_i2c.c
@@ -395,7 +395,7 @@ static int saa7146_i2c_xfer(struct i2c_adapter* adapter, 
struct i2c_msg *msg, in
 /* i2c-adapter helper functions  */
 
 /* exported algorithm data */
-static struct i2c_algorithm saa7146_algo = {
+static const struct i2c_algorithm saa7146_algo = {
.master_xfer= saa7146_i2c_xfer,
.functionality  = saa7146_i2c_func,
 };
-- 
2.5.0

[PATCH] dib9000: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/dvb-frontends/dib9000.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-frontends/dib9000.c 
b/drivers/media/dvb-frontends/dib9000.c
index c95fff4..17c6f15 100644
--- a/drivers/media/dvb-frontends/dib9000.c
+++ b/drivers/media/dvb-frontends/dib9000.c
@@ -1714,12 +1714,12 @@ static u32 dib9000_i2c_func(struct i2c_adapter *adapter)
return I2C_FUNC_I2C;
 }
 
-static struct i2c_algorithm dib9000_tuner_algo = {
+static const struct i2c_algorithm dib9000_tuner_algo = {
.master_xfer = dib9000_tuner_xfer,
.functionality = dib9000_i2c_func,
 };
 
-static struct i2c_algorithm dib9000_component_bus_algo = {
+static const struct i2c_algorithm dib9000_component_bus_algo = {
.master_xfer = dib9000_fw_component_bus_xfer,
.functionality = dib9000_i2c_func,
 };
-- 
2.5.0

[PATCH] usbvision: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/usb/usbvision/usbvision-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/usbvision/usbvision-i2c.c 
b/drivers/media/usb/usbvision/usbvision-i2c.c
index fdf6b6e..f86a0e0 100644
--- a/drivers/media/usb/usbvision/usbvision-i2c.c
+++ b/drivers/media/usb/usbvision/usbvision-i2c.c
@@ -163,7 +163,7 @@ static u32 functionality(struct i2c_adapter *adap)
 
 /* -exported algorithm data: - */
 
-static struct i2c_algorithm usbvision_algo = {
+static const struct i2c_algorithm usbvision_algo = {
.master_xfer   = usbvision_i2c_xfer,
.smbus_xfer= NULL,
.functionality = functionality,
-- 
2.5.0

[PATCH] dib7000p: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/dvb-frontends/dib7000p.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/dib7000p.c 
b/drivers/media/dvb-frontends/dib7000p.c
index 1caa04d..0fbaabe 100644
--- a/drivers/media/dvb-frontends/dib7000p.c
+++ b/drivers/media/dvb-frontends/dib7000p.c
@@ -2388,7 +2388,7 @@ static u32 dib7000p_i2c_func(struct i2c_adapter *adapter)
return I2C_FUNC_I2C;
 }
 
-static struct i2c_algorithm dib7090_tuner_xfer_algo = {
+static const struct i2c_algorithm dib7090_tuner_xfer_algo = {
.master_xfer = dib7090_tuner_xfer,
.functionality = dib7000p_i2c_func,
 };
-- 
2.5.0

[PATCH] dvb-ttusb-budget: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index 361e40b..22a488d 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -1640,7 +1640,7 @@ static void frontend_init(struct ttusb* ttusb)
 
 
 
-static struct i2c_algorithm ttusb_dec_algo = {
+static const struct i2c_algorithm ttusb_dec_algo = {
.master_xfer= master_xfer,
.functionality  = functionality,
 };
-- 
2.5.0

Re: [PATCH] rcar_fdp1: constify vb2_ops structure

[Gustavo A. R. Silva]

Hi Kieran,

Quoting Kieran Bingham <kieran.bingham+rene...@ideasonboard.com>:


Hi Gustavo,

Thank you for the patch.



Absolutely, glad to help. :)


On 06/07/17 21:25, Gustavo A. R. Silva wrote:

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>


Reviewed-by: Kieran Bingham <kieran.bingham+rene...@ideasonboard.com>


---
 drivers/media/platform/rcar_fdp1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/rcar_fdp1.c  
b/drivers/media/platform/rcar_fdp1.c

index 3ee51fc..3245bc4 100644
--- a/drivers/media/platform/rcar_fdp1.c
+++ b/drivers/media/platform/rcar_fdp1.c
@@ -2032,7 +2032,7 @@ static void fdp1_stop_streaming(struct vb2_queue *q)
}
 }

-static struct vb2_ops fdp1_qops = {
+static const struct vb2_ops fdp1_qops = {
.queue_setup = fdp1_queue_setup,
.buf_prepare = fdp1_buf_prepare,
.buf_queue   = fdp1_buf_queue,



--
Gustavo A. R. Silva

[PATCH] ddbridge: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
b/drivers/media/pci/ddbridge/ddbridge-core.c
index cd1723e..9663a4c 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -200,7 +200,7 @@ static u32 ddb_i2c_functionality(struct i2c_adapter *adap)
return I2C_FUNC_SMBUS_EMUL;
 }
 
-static struct i2c_algorithm ddb_i2c_algo = {
+static const struct i2c_algorithm ddb_i2c_algo = {
.master_xfer   = ddb_i2c_master_xfer,
.functionality = ddb_i2c_functionality,
 };
-- 
2.5.0

[PATCH] dib8000: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/dvb-frontends/dib8000.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/dib8000.c 
b/drivers/media/dvb-frontends/dib8000.c
index e501ec9..a179a3f 100644
--- a/drivers/media/dvb-frontends/dib8000.c
+++ b/drivers/media/dvb-frontends/dib8000.c
@@ -1880,7 +1880,7 @@ static u32 dib8096p_i2c_func(struct i2c_adapter *adapter)
return I2C_FUNC_I2C;
 }
 
-static struct i2c_algorithm dib8096p_tuner_xfer_algo = {
+static const struct i2c_algorithm dib8096p_tuner_xfer_algo = {
.master_xfer = dib8096p_tuner_xfer,
.functionality = dib8096p_i2c_func,
 };
-- 
2.5.0

[PATCH] cx24123: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/dvb-frontends/cx24123.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/cx24123.c 
b/drivers/media/dvb-frontends/cx24123.c
index 4ae3d92..1d59d1d 100644
--- a/drivers/media/dvb-frontends/cx24123.c
+++ b/drivers/media/dvb-frontends/cx24123.c
@@ -1032,7 +1032,7 @@ static u32 cx24123_tuner_i2c_func(struct i2c_adapter 
*adapter)
return I2C_FUNC_I2C;
 }
 
-static struct i2c_algorithm cx24123_tuner_i2c_algo = {
+static const struct i2c_algorithm cx24123_tuner_i2c_algo = {
.master_xfer   = cx24123_tuner_i2c_tuner_xfer,
.functionality = cx24123_tuner_i2c_func,
 };
-- 
2.5.0

[PATCH] zd1301_demod: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/dvb-frontends/zd1301_demod.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/zd1301_demod.c 
b/drivers/media/dvb-frontends/zd1301_demod.c
index fcf5f69..84a2b25 100644
--- a/drivers/media/dvb-frontends/zd1301_demod.c
+++ b/drivers/media/dvb-frontends/zd1301_demod.c
@@ -445,7 +445,7 @@ static u32 zd1301_demod_i2c_functionality(struct 
i2c_adapter *adapter)
return I2C_FUNC_I2C;
 }
 
-static struct i2c_algorithm zd1301_demod_i2c_algorithm = {
+static const struct i2c_algorithm zd1301_demod_i2c_algorithm = {
.master_xfer   = zd1301_demod_i2c_master_xfer,
.functionality = zd1301_demod_i2c_functionality,
 };
-- 
2.5.0

[PATCH] ngene: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/pci/ngene/ngene-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/ngene/ngene-i2c.c 
b/drivers/media/pci/ngene/ngene-i2c.c
index fbf3635..3004947 100644
--- a/drivers/media/pci/ngene/ngene-i2c.c
+++ b/drivers/media/pci/ngene/ngene-i2c.c
@@ -150,7 +150,7 @@ static u32 ngene_i2c_functionality(struct i2c_adapter *adap)
return I2C_FUNC_SMBUS_EMUL;
 }
 
-static struct i2c_algorithm ngene_i2c_algo = {
+static const struct i2c_algorithm ngene_i2c_algo = {
.master_xfer = ngene_i2c_master_xfer,
.functionality = ngene_i2c_functionality,
 };
-- 
2.5.0

[PATCH] rcar_fdp1: constify vb2_ops structure

[Gustavo A. R. Silva]

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/rcar_fdp1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/rcar_fdp1.c 
b/drivers/media/platform/rcar_fdp1.c
index 3ee51fc..3245bc4 100644
--- a/drivers/media/platform/rcar_fdp1.c
+++ b/drivers/media/platform/rcar_fdp1.c
@@ -2032,7 +2032,7 @@ static void fdp1_stop_streaming(struct vb2_queue *q)
}
 }
 
-static struct vb2_ops fdp1_qops = {
+static const struct vb2_ops fdp1_qops = {
.queue_setup = fdp1_queue_setup,
.buf_prepare = fdp1_buf_prepare,
.buf_queue   = fdp1_buf_queue,
-- 
2.5.0

[PATCH] stm32-dcmi: constify vb2_ops structure

[Gustavo A. R. Silva]

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/stm32/stm32-dcmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 83d32a5..24ef888 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -662,7 +662,7 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
dcmi->errors_count, dcmi->buffers_count);
 }
 
-static struct vb2_ops dcmi_video_qops = {
+static const struct vb2_ops dcmi_video_qops = {
.queue_setup= dcmi_queue_setup,
.buf_init   = dcmi_buf_init,
.buf_prepare= dcmi_buf_prepare,
-- 
2.5.0

[PATCH] atmel-isc: constify vb2_ops structure

[Gustavo A. R. Silva]

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/atmel/atmel-isc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index d653425..d4df3d4 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -873,7 +873,7 @@ static void isc_buffer_queue(struct vb2_buffer *vb)
spin_unlock_irqrestore(>dma_queue_lock, flags);
 }
 
-static struct vb2_ops isc_vb2_ops = {
+static const struct vb2_ops isc_vb2_ops = {
.queue_setup= isc_queue_setup,
.wait_prepare   = vb2_ops_wait_prepare,
.wait_finish= vb2_ops_wait_finish,
-- 
2.5.0

[PATCH] mediatek: constify vb2_ops structure

[Gustavo A. R. Silva]

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c 
b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index 451a540..f17a86b 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -756,7 +756,7 @@ static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
pm_runtime_put_sync(ctx->jpeg->dev);
 }
 
-static struct vb2_ops mtk_jpeg_qops = {
+static const struct vb2_ops mtk_jpeg_qops = {
.queue_setup= mtk_jpeg_queue_setup,
.buf_prepare= mtk_jpeg_buf_prepare,
.buf_queue  = mtk_jpeg_buf_queue,
-- 
2.5.0

[PATCH] davinci: vpif_display: constify vb2_ops structure

[Gustavo A. R. Silva]

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/davinci/vpif_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_display.c 
b/drivers/media/platform/davinci/vpif_display.c
index b5ac6ce..1a494d3 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -290,7 +290,7 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
spin_unlock_irqrestore(>irqlock, flags);
 }
 
-static struct vb2_ops video_qops = {
+static const struct vb2_ops video_qops = {
.queue_setup= vpif_buffer_queue_setup,
.wait_prepare   = vb2_ops_wait_prepare,
.wait_finish= vb2_ops_wait_finish,
-- 
2.5.0

[PATCH] mtk-mdp: constify vb2_ops structure

[Gustavo A. R. Silva]

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c 
b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
index 13afe48..3038d62 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
@@ -621,7 +621,7 @@ static void mtk_mdp_m2m_buf_queue(struct vb2_buffer *vb)
v4l2_m2m_buf_queue(ctx->m2m_ctx, to_vb2_v4l2_buffer(vb));
 }
 
-static struct vb2_ops mtk_mdp_m2m_qops = {
+static const struct vb2_ops mtk_mdp_m2m_qops = {
.queue_setup = mtk_mdp_m2m_queue_setup,
.buf_prepare = mtk_mdp_m2m_buf_prepare,
.buf_queue   = mtk_mdp_m2m_buf_queue,
-- 
2.5.0

[PATCH] davinci: vpif_capture: constify vb2_ops structure

[Gustavo A. R. Silva]

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index d78580f..67fa53d 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -312,7 +312,7 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
spin_unlock_irqrestore(>irqlock, flags);
 }
 
-static struct vb2_ops video_qops = {
+static const struct vb2_ops video_qops = {
.queue_setup= vpif_buffer_queue_setup,
.buf_prepare= vpif_buffer_prepare,
.start_streaming= vpif_start_streaming,
-- 
2.5.0

[PATCH] pxa_camera: constify vb2_ops structure

[Gustavo A. R. Silva]

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/pxa_camera.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/pxa_camera.c 
b/drivers/media/platform/pxa_camera.c
index 3990951..5c3b7cf 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -1557,7 +1557,7 @@ static void pxac_vb2_stop_streaming(struct vb2_queue *vq)
pxa_camera_wakeup(pcdev, buf, VB2_BUF_STATE_ERROR);
 }
 
-static struct vb2_ops pxac_vb2_ops = {
+static const struct vb2_ops pxac_vb2_ops = {
.queue_setup= pxac_vb2_queue_setup,
.buf_init   = pxac_vb2_init,
.buf_prepare= pxac_vb2_prepare,
-- 
2.5.0

[PATCH] dm1105: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/pci/dm1105/dm1105.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/dm1105/dm1105.c 
b/drivers/media/pci/dm1105/dm1105.c
index 1d41934..36e94f8 100644
--- a/drivers/media/pci/dm1105/dm1105.c
+++ b/drivers/media/pci/dm1105/dm1105.c
@@ -571,7 +571,7 @@ static u32 functionality(struct i2c_adapter *adap)
return I2C_FUNC_I2C;
 }
 
-static struct i2c_algorithm dm1105_algo = {
+static const struct i2c_algorithm dm1105_algo = {
.master_xfer   = dm1105_i2c_xfer,
.functionality = functionality,
 };
-- 
2.5.0

[PATCH] mantis: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/pci/mantis/mantis_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/mantis/mantis_i2c.c 
b/drivers/media/pci/mantis/mantis_i2c.c
index d72ee47..496c10d 100644
--- a/drivers/media/pci/mantis/mantis_i2c.c
+++ b/drivers/media/pci/mantis/mantis_i2c.c
@@ -212,7 +212,7 @@ static u32 mantis_i2c_func(struct i2c_adapter *adapter)
return I2C_FUNC_SMBUS_EMUL;
 }
 
-static struct i2c_algorithm mantis_algo = {
+static const struct i2c_algorithm mantis_algo = {
.master_xfer= mantis_i2c_xfer,
.functionality  = mantis_i2c_func,
 };
-- 
2.5.0

Re: [PATCH] ddbridge: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Hi Daniel,


On 07/10/2017 10:16 AM, Daniel Scheller wrote:

Am Sun, 9 Jul 2017 20:15:36 -0500
schrieb "Gustavo A. R. Silva" <garsi...@embeddedor.com>:


Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
  drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c
b/drivers/media/pci/ddbridge/ddbridge-core.c index cd1723e..9663a4c
100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -200,7 +200,7 @@ static u32 ddb_i2c_functionality(struct
i2c_adapter *adap) return I2C_FUNC_SMBUS_EMUL;
  }
  
-static struct i2c_algorithm ddb_i2c_algo = {

+static const struct i2c_algorithm ddb_i2c_algo = {
.master_xfer   = ddb_i2c_master_xfer,
.functionality = ddb_i2c_functionality,
  };

Hi Gustavo,
Hi all,

please hold this single one patch from the constify patches back for
now, since we're in the process of bumping the whole driver to a newer
version which involves lots of code shuffling. With this, quite some
GIT rebasing work needs to be done, and adding this one liner at a
later time (thus rebasing it) is way easier.

To be sure this will not be forgotten afterwards, I've already posted a
patch applying the exact change at [1].

Thank you very much!

[1] https://patchwork.linuxtv.org/patch/42393/


Thank you!

--
Gustavo A. R. Silva

Re: [PATCH] stm32-dcmi: constify vb2_ops structure

[Gustavo A. R. Silva]

On 07/07/2017 09:33 AM, Hugues FRUCHET wrote:

Acked-by: Hugues Fruchet <hugues.fruc...@st.com>


Thank you, Hugues.


On 07/06/2017 10:05 PM, Gustavo A. R. Silva wrote:

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
   drivers/media/platform/stm32/stm32-dcmi.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 83d32a5..24ef888 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -662,7 +662,7 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
dcmi->errors_count, dcmi->buffers_count);
   }
   
-static struct vb2_ops dcmi_video_qops = {

+static const struct vb2_ops dcmi_video_qops = {
.queue_setup= dcmi_queue_setup,
.buf_init   = dcmi_buf_init,
.buf_prepare= dcmi_buf_prepare,


--
Gustavo A. R. Silva

Re: [PATCH] st-delta: constify vb2_ops structures

[Gustavo A. R. Silva]

Thank you, Hugues.


On 07/07/2017 09:33 AM, Hugues FRUCHET wrote:

Acked-by: Hugues Fruchet <hugues.fruc...@st.com>

On 07/06/2017 10:14 PM, Gustavo A. R. Silva wrote:

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
   drivers/media/platform/sti/delta/delta-v4l2.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c 
b/drivers/media/platform/sti/delta/delta-v4l2.c
index c6f2e24..ff9850e 100644
--- a/drivers/media/platform/sti/delta/delta-v4l2.c
+++ b/drivers/media/platform/sti/delta/delta-v4l2.c
@@ -1574,7 +1574,7 @@ static void delta_vb2_frame_stop_streaming(struct 
vb2_queue *q)
   }
   
   /* VB2 queue ops */

-static struct vb2_ops delta_vb2_au_ops = {
+static const struct vb2_ops delta_vb2_au_ops = {
.queue_setup = delta_vb2_au_queue_setup,
.buf_prepare = delta_vb2_au_prepare,
.buf_queue = delta_vb2_au_queue,
@@ -1584,7 +1584,7 @@ static struct vb2_ops delta_vb2_au_ops = {
.stop_streaming = delta_vb2_au_stop_streaming,
   };
   
-static struct vb2_ops delta_vb2_frame_ops = {

+static const struct vb2_ops delta_vb2_frame_ops = {
.queue_setup = delta_vb2_frame_queue_setup,
.buf_prepare = delta_vb2_frame_prepare,
.buf_finish = delta_vb2_frame_finish,


--
Gustavo A. R. Silva

[PATCH] s5k5baf: remove unnecessary static in s5k5baf_get_selection()

[Gustavo A. R. Silva]

Remove unnecessary static on local variable rtype.
Such variable is initialized before being used,
on every execution path throughout the function.
The static has no benefit and, removing it reduces
the code size.

This issue was detected using Coccinelle and the following semantic patch:

@bad exists@
position p;
identifier x;
type T;
@@

static T x@p;
...
x = <+...x...+>

@@
identifier x;
expression e;
type T;
position p != bad.p;
@@

-static
 T x@p;
 ... when != x
 when strict
?x = e;

In the following log you can see the difference in the code size. Also,
there is a significant difference in the bss segment. This log is the
output of the size command, before and after the code change:

before:
   textdata bss dec hex filename
  277655656 320   3374183cd drivers/media/i2c/s5k5baf.o

after:
   textdata bss dec hex filename
  277335600 256   335898335 drivers/media/i2c/s5k5baf.o


Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/i2c/s5k5baf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index 962051b..f01722d 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -1374,7 +1374,7 @@ static int s5k5baf_get_selection(struct v4l2_subdev *sd,
 struct v4l2_subdev_pad_config *cfg,
 struct v4l2_subdev_selection *sel)
 {
-   static enum selection_rect rtype;
+   enum selection_rect rtype;
struct s5k5baf *state = to_s5k5baf(sd);
 
rtype = s5k5baf_get_sel_rect(sel->pad, sel->target);
-- 
2.5.0

[PATCH] st-delta: constify vb2_ops structures

[Gustavo A. R. Silva]

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/sti/delta/delta-v4l2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c 
b/drivers/media/platform/sti/delta/delta-v4l2.c
index c6f2e24..ff9850e 100644
--- a/drivers/media/platform/sti/delta/delta-v4l2.c
+++ b/drivers/media/platform/sti/delta/delta-v4l2.c
@@ -1574,7 +1574,7 @@ static void delta_vb2_frame_stop_streaming(struct 
vb2_queue *q)
 }
 
 /* VB2 queue ops */
-static struct vb2_ops delta_vb2_au_ops = {
+static const struct vb2_ops delta_vb2_au_ops = {
.queue_setup = delta_vb2_au_queue_setup,
.buf_prepare = delta_vb2_au_prepare,
.buf_queue = delta_vb2_au_queue,
@@ -1584,7 +1584,7 @@ static struct vb2_ops delta_vb2_au_ops = {
.stop_streaming = delta_vb2_au_stop_streaming,
 };
 
-static struct vb2_ops delta_vb2_frame_ops = {
+static const struct vb2_ops delta_vb2_frame_ops = {
.queue_setup = delta_vb2_frame_queue_setup,
.buf_prepare = delta_vb2_frame_prepare,
.buf_finish = delta_vb2_frame_finish,
-- 
2.5.0

[PATCH] sir_ir: remove unnecessary static in sir_interrupt()

[Gustavo A. R. Silva]

Remove unnecessary static on local variable delt.
Such variable is initialized before being used,
on every execution path throughout the function.
The static has no benefit and, removing it reduces
the code size.

This issue was detected using Coccinelle and the following semantic patch:

@bad exists@
position p;
identifier x;
type T;
@@

static T x@p;
...
x = <+...x...+>

@@
identifier x;
expression e;
type T;
position p != bad.p;
@@

-static
 T x@p;
 ... when != x
 when strict
?x = e;

In the following log you can see the difference in the code size. Also,
there is a significant difference in the bss segment. This log is the
output of the size command, before and after the code change:

before:
   textdata bss dec hex filename
   50093456 57690412351 drivers/media/rc/sir_ir.o

after:
   textdata bss dec hex filename
   49883400 512890022c4 drivers/media/rc/sir_ir.o

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/rc/sir_ir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/sir_ir.c b/drivers/media/rc/sir_ir.c
index 20234ba..be67f7c 100644
--- a/drivers/media/rc/sir_ir.c
+++ b/drivers/media/rc/sir_ir.c
@@ -155,7 +155,7 @@ static irqreturn_t sir_interrupt(int irq, void *dev_id)
 {
unsigned char data;
ktime_t curr_time;
-   static unsigned long delt;
+   unsigned long delt;
unsigned long deltintr;
unsigned long flags;
int counter = 0;
-- 
2.5.0

[PATCH] tuners: remove unnecessary static in simple_dvb_configure()

[Gustavo A. R. Silva]

Remove unnecessary static on local variable t_params.
Such variable is initialized before being used,
on every execution path throughout the function.
The static has no benefit and, removing it reduces
the code size.

This issue was detected using Coccinelle and the following semantic patch:

@bad exists@
position p;
identifier x;
type T;
@@

static T x@p;
...
x = <+...x...+>

@@
identifier x;
expression e;
type T;
position p != bad.p;
@@

-static
 T x@p;
 ... when != x
 when strict
?x = e;

In the following log you can see the difference in the code size. Also,
there is a significant difference in the bss segment. This log is the
output of the size command, before and after the code change:

before:
   textdata bss dec hex filename
  233143640 832   277866c8a drivers/media/tuners/tuner-simple.o

after:
   textdata bss dec hex filename
  232573552 768   275776bb9 drivers/media/tuners/tuner-simple.o

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/tuners/tuner-simple.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/tuners/tuner-simple.c 
b/drivers/media/tuners/tuner-simple.c
index 3339b13..cf44d36 100644
--- a/drivers/media/tuners/tuner-simple.c
+++ b/drivers/media/tuners/tuner-simple.c
@@ -846,7 +846,7 @@ static u32 simple_dvb_configure(struct dvb_frontend *fe, u8 
*buf,
/* This function returns the tuned frequency on success, 0 on error */
struct tuner_simple_priv *priv = fe->tuner_priv;
struct tunertype *tun = priv->tun;
-   static struct tuner_params *t_params;
+   struct tuner_params *t_params;
u8 config, cb;
u32 div;
int ret;
-- 
2.5.0

[PATCH] media: venus: fix duplicated code for different branches

[Gustavo A. R. Silva]

Refactor code in order to avoid identical code for different branches.

This issue was detected with the help of Coccinelle.

Addresses-Coverity-ID: 1415317
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
This code was reported by Coverity and it was tested by compilation only.
Please, verify if this is an actual bug.

 drivers/media/platform/qcom/venus/helpers.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/helpers.c 
b/drivers/media/platform/qcom/venus/helpers.c
index 5f4434c..8a5c467 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -240,11 +240,7 @@ static void return_buf_error(struct venus_inst *inst,
 {
struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
 
-   if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
-   v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
-   else
-   v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
-
+   v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }
 
-- 
2.5.0

[PATCH] dib0090: fix duplicated code for different branches

[Gustavo A. R. Silva]

Refactor code in order to avoid identical code for different branches.

This issue was detected with the help of Coccinelle.

Addresses-Coverity-ID: 1226795
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
This code was tested by compilation only.

 drivers/media/dvb-frontends/dib0090.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/media/dvb-frontends/dib0090.c 
b/drivers/media/dvb-frontends/dib0090.c
index ae53a19..d9d730d 100644
--- a/drivers/media/dvb-frontends/dib0090.c
+++ b/drivers/media/dvb-frontends/dib0090.c
@@ -2435,14 +2435,7 @@ static int dib0090_tune(struct dvb_frontend *fe)
Den = 1;
 
if (Rest > 0) {
-   if (state->config->analog_output)
-   lo6 |= (1 << 2) | 2;
-   else {
-   if (state->identity.in_soc)
-   lo6 |= (1 << 2) | 2;
-   else
-   lo6 |= (1 << 2) | 2;
-   }
+   lo6 |= (1 << 2) | 2;
Den = 255;
}
dib0090_write_reg(state, 0x15, (u16) FBDiv);
-- 
2.5.0

Re: [PATCH v2] venus: fix copy/paste error in return_buf_error

[Gustavo A. R. Silva]

On 08/21/2017 04:14 AM, Stanimir Varbanov wrote:

Thanks Gustavo!



Glad to help. :)


On 08/18/2017 07:07 PM, Gustavo A. R. Silva wrote:

Call function v4l2_m2m_dst_buf_remove_by_buf() instead of
v4l2_m2m_src_buf_remove_by_buf()

Addresses-Coverity-ID: 1415317
Cc: Stanimir Varbanov <stanimir.varba...@linaro.org>
Cc: Hans Verkuil <hverk...@xs4all.nl>
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
Changes in v2:
 Stanimir Varbanov confirmed this is a bug. The correct fix is to call
 function v4l2_m2m_dst_buf_remove_by_buf instead of function
 v4l2_m2m_src_buf_remove_by_buf in the _else_ branch.

 drivers/media/platform/qcom/venus/helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


Acked-by: Stanimir Varbanov <stanimir.varba...@linaro.org>



diff --git a/drivers/media/platform/qcom/venus/helpers.c 
b/drivers/media/platform/qcom/venus/helpers.c
index 5f4434c..2d61879 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -243,7 +243,7 @@ static void return_buf_error(struct venus_inst *inst,
if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
else
-   v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
+   v4l2_m2m_dst_buf_remove_by_buf(m2m_ctx, vbuf);

v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }





--
Gustavo A. R. Silva

[PATCH] media: i2c: initialize scalar variables

[Gustavo A. R. Silva]

Initialize scalar variables _pid_ and _ver_ to avoid a possible misbehavior.

Addresses-Coverity-ID: 1324239
Addresses-Coverity-ID: 1324240
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/i2c/ov2659.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
index 6e63672..58615f8 100644
--- a/drivers/media/i2c/ov2659.c
+++ b/drivers/media/i2c/ov2659.c
@@ -1308,7 +1308,8 @@ static const struct v4l2_subdev_internal_ops 
ov2659_subdev_internal_ops = {
 static int ov2659_detect(struct v4l2_subdev *sd)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
-   u8 pid, ver;
+   u8 pid = 0;
+   u8 ver = 0;
int ret;
 
dev_dbg(>dev, "%s:\n", __func__);
-- 
2.5.0

[PATCH] media: usb: au0828: remove dead code

[Gustavo A. R. Silva]

Local variable _ret_ is assigned to a constant value and it is never
updated again. Remove this variable and the dead code it guards.

Addresses-Coverity-ID: 112968
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/usb/au0828/au0828-video.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c 
b/drivers/media/usb/au0828/au0828-video.c
index 16f9125..abc80b0 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -809,16 +809,10 @@ static void au0828_analog_stream_reset(struct au0828_dev 
*dev)
  */
 static int au0828_stream_interrupt(struct au0828_dev *dev)
 {
-   int ret = 0;
-
dev->stream_state = STREAM_INTERRUPT;
if (test_bit(DEV_DISCONNECTED, >dev_state))
return -ENODEV;
-   else if (ret) {
-   set_bit(DEV_MISCONFIGURED, >dev_state);
-   dprintk(1, "%s device is misconfigured!\n", __func__);
-   return ret;
-   }
+
return 0;
 }
 
-- 
2.5.0

[PATCH] media: platform: coda: remove variable self assignment

[Gustavo A. R. Silva]

Remove variable self assignment.

Addresses-Coverity-ID: 1408817
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/coda/coda-common.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/coda/coda-common.c 
b/drivers/media/platform/coda/coda-common.c
index d523e99..a6699d8 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -612,7 +612,6 @@ static int coda_try_fmt_vid_cap(struct file *file, void 
*priv,
 
/* The h.264 decoder only returns complete 16x16 macroblocks */
if (codec && codec->src_fourcc == V4L2_PIX_FMT_H264) {
-   f->fmt.pix.width = f->fmt.pix.width;
f->fmt.pix.height = round_up(f->fmt.pix.height, 16);
f->fmt.pix.bytesperline = round_up(f->fmt.pix.width, 16);
f->fmt.pix.sizeimage = f->fmt.pix.bytesperline *
-- 
2.5.0

Re: [media-pci-cx25821] question about value overwrite

[Gustavo A. R. Silva]

Hi Hans,

Quoting Hans Verkuil <hverk...@xs4all.nl>:


On 05/19/2017 12:07 AM, Gustavo A. R. Silva wrote:


Hello everybody,

While looking into Coverity ID 1226903 I ran into the following piece
of code at drivers/media/pci/cx25821/cx25821-medusa-video.c:393:

393int medusa_set_videostandard(struct cx25821_dev *dev)
394{
395int status = 0;
396u32 value = 0, tmp = 0;
397
398if (dev->tvnorm & V4L2_STD_PAL_BG || dev->tvnorm &  
V4L2_STD_PAL_DK)

399status = medusa_initialize_pal(dev);
400else
401status = medusa_initialize_ntsc(dev);
402
403/* Enable DENC_A output */
404value = cx25821_i2c_read(>i2c_bus[0], DENC_A_REG_4, );
405value = setBitAtPos(value, 4);
406status = cx25821_i2c_write(>i2c_bus[0],  
DENC_A_REG_4, value);

407
408/* Enable DENC_B output */
409value = cx25821_i2c_read(>i2c_bus[0], DENC_B_REG_4, );
410value = setBitAtPos(value, 4);
411status = cx25821_i2c_write(>i2c_bus[0],  
DENC_B_REG_4, value);

412
413return status;
414}

The issue is that the value stored in variable _status_ at lines 399
and 401 is overwritten by the one stored at line 406 and then at line
411, before it can be used.

My question is if the original intention was to ORed the return
values, something like in the following patch:

index 0a9db05..226d14f 100644
--- a/drivers/media/pci/cx25821/cx25821-medusa-video.c
+++ b/drivers/media/pci/cx25821/cx25821-medusa-video.c
@@ -403,12 +403,12 @@ int medusa_set_videostandard(struct cx25821_dev *dev)
 /* Enable DENC_A output */
 value = cx25821_i2c_read(>i2c_bus[0], DENC_A_REG_4, );
 value = setBitAtPos(value, 4);
-   status = cx25821_i2c_write(>i2c_bus[0], DENC_A_REG_4, value);
+   status |= cx25821_i2c_write(>i2c_bus[0], DENC_A_REG_4, value);

 /* Enable DENC_B output */
 value = cx25821_i2c_read(>i2c_bus[0], DENC_B_REG_4, );
 value = setBitAtPos(value, 4);
-   status = cx25821_i2c_write(>i2c_bus[0], DENC_B_REG_4, value);
+   status |= cx25821_i2c_write(>i2c_bus[0], DENC_B_REG_4, value);

 return status;
  }


This is a crappy driver, they just couldn't be bothered to check the  
error from

cx25821_i2c_read/write.

Strictly speaking the return value should be checked after every  
read/write and

returned in case of an error.



Yeah, the same happens in functions medusa_initialize_pal() and  
medusa_initialize_ntsc()



Not sure whether it is worth the effort fixing this.



Thank you for your reply.
--
Gustavo A. R. Silva

[PATCH] radio: wl1273: add check on core->write() return value

[Gustavo A. R. Silva]

Check return value from call to core->write(), so in case of
error print error message, jump to goto label fail and eventually
return.

Addresses-Coverity-ID: 1226943
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/radio/radio-wl1273.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/media/radio/radio-wl1273.c 
b/drivers/media/radio/radio-wl1273.c
index 7240223..17e82a9 100644
--- a/drivers/media/radio/radio-wl1273.c
+++ b/drivers/media/radio/radio-wl1273.c
@@ -610,10 +610,21 @@ static int wl1273_fm_start(struct wl1273_device *radio, 
int new_mode)
}
}
 
-   if (radio->rds_on)
+   if (radio->rds_on) {
r = core->write(core, WL1273_RDS_DATA_ENB, 1);
-   else
+   if (r) {
+   dev_err(dev, "%s: RDS_DATA_ENB ON fails\n",
+   __func__);
+   goto fail;
+   }
+   } else {
r = core->write(core, WL1273_RDS_DATA_ENB, 0);
+   if (r) {
+   dev_err(dev, "%s: RDS_DATA_ENB OFF fails\n",
+   __func__);
+   goto fail;
+   }
+   }
} else {
dev_warn(dev, "%s: Illegal mode.\n", __func__);
}
-- 
2.5.0

[PATCH] tuners: mxl5005s: remove useless variable assignments

[Gustavo A. R. Silva]

Values assigned to variables Fmax and Fmin at lines 2740 and 2741 are
overwritten at lines 2754 and 2755 before they can be used. This makes
such variable assignments useless.

Addresses-Coverity-ID: 1226952
Addresses-Coverity-ID: 1226953
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/tuners/mxl5005s.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/tuners/mxl5005s.c b/drivers/media/tuners/mxl5005s.c
index 353744f..dd59c2c 100644
--- a/drivers/media/tuners/mxl5005s.c
+++ b/drivers/media/tuners/mxl5005s.c
@@ -2737,8 +2737,6 @@ static u16 MXL_TuneRF(struct dvb_frontend *fe, u32 
RF_Freq)
status += MXL_ControlWrite(fe, TG_LO_DIVVAL,0x0);
status += MXL_ControlWrite(fe, TG_LO_SELVAL,0x7);
divider_val = 2 ;
-   Fmax = FmaxBin ;
-   Fmin = FminBin ;
}
 
/* TG_DIV_VAL */
-- 
2.5.0

[PATCH] dvb-usb-v2: lmedm04: remove unnecessary variable in lme2510_stream_restart()

[Gustavo A. R. Silva]

Remove unnecessary variable _ret_ and refactor the code.

Addresses-Coverity-ID: 1226934
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/usb/dvb-usb-v2/lmedm04.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c 
b/drivers/media/usb/dvb-usb-v2/lmedm04.c
index 594360a..a91fdad 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -207,15 +207,13 @@ static int lme2510_stream_restart(struct dvb_usb_device 
*d)
struct lme2510_state *st = d->priv;
u8 all_pids[] = LME_ALL_PIDS;
u8 stream_on[] = LME_ST_ON_W;
-   int ret;
u8 rbuff[1];
if (st->pid_off)
-   ret = lme2510_usb_talk(d, all_pids, sizeof(all_pids),
-   rbuff, sizeof(rbuff));
+   lme2510_usb_talk(d, all_pids, sizeof(all_pids),
+rbuff, sizeof(rbuff));
/*Restart Stream Command*/
-   ret = lme2510_usb_talk(d, stream_on, sizeof(stream_on),
-   rbuff, sizeof(rbuff));
-   return ret;
+   return lme2510_usb_talk(d, stream_on, sizeof(stream_on),
+   rbuff, sizeof(rbuff));
 }
 
 static int lme2510_enable_pid(struct dvb_usb_device *d, u8 index, u16 pid_out)
-- 
2.5.0

[PATCH] i2c: tvp5150: remove useless variable assignment in tvp5150_set_vbi()

[Gustavo A. R. Silva]

Value assigned to variable _type_ at line 678 is overwritten at line 688
before it can be used. This makes such variable assignment useless.

Remove this variable assignment and fix some coding style issues.

Addresses-Coverity-ID: 1226968
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/i2c/tvp5150.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 04e96b3..2fcd2e5 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -658,7 +658,7 @@ static int tvp5150_set_vbi(struct v4l2_subdev *sd,
struct tvp5150 *decoder = to_tvp5150(sd);
v4l2_std_id std = decoder->norm;
u8 reg;
-   int pos=0;
+   int pos = 0;
 
if (std == V4L2_STD_ALL) {
dev_err(sd->dev, "VBI can't be configured without knowing 
number of lines\n");
@@ -668,33 +668,30 @@ static int tvp5150_set_vbi(struct v4l2_subdev *sd,
line += 3;
}
 
-   if (line<6||line>27)
+   if (line < 6 || line > 27)
return 0;
 
-   while (regs->reg != (u16)-1 ) {
+   while (regs->reg != (u16)-1) {
if ((type & regs->type.vbi_type) &&
-   (line>=regs->type.ini_line) &&
-   (line<=regs->type.end_line)) {
-   type=regs->type.vbi_type;
+   (line >= regs->type.ini_line) &&
+   (line <= regs->type.end_line))
break;
-   }
 
regs++;
pos++;
}
+
if (regs->reg == (u16)-1)
return 0;
 
-   type=pos | (flags & 0xf0);
-   reg=((line-6)<<1)+TVP5150_LINE_MODE_INI;
+   type = pos | (flags & 0xf0);
+   reg = ((line - 6) << 1) + TVP5150_LINE_MODE_INI;
 
-   if (fields&1) {
+   if (fields & 1)
tvp5150_write(sd, reg, type);
-   }
 
-   if (fields&2) {
-   tvp5150_write(sd, reg+1, type);
-   }
+   if (fields & 2)
+   tvp5150_write(sd, reg + 1, type);
 
return type;
 }
-- 
2.5.0

[PATCH] dvb-frontends: mb86a16: remove useless variables in signal_det()

[Gustavo A. R. Silva]

Remove useless variables wait_t and wait_sym and code related.

Also, fix some coding style issues.

Addresses-Coverity-ID: 1226947
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/dvb-frontends/mb86a16.c | 25 +++--
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/media/dvb-frontends/mb86a16.c 
b/drivers/media/dvb-frontends/mb86a16.c
index 9bb122c..dfe322e 100644
--- a/drivers/media/dvb-frontends/mb86a16.c
+++ b/drivers/media/dvb-frontends/mb86a16.c
@@ -415,27 +415,21 @@ static int signal_det(struct mb86a16_state *state,
  int smrt,
  unsigned char *SIG)
 {
-
-   int ret ;
-   int smrtd ;
-   int wait_sym ;
-
-   u32 wait_t;
-   unsigned char S[3] ;
-   int i ;
+   int ret;
+   int smrtd;
+   unsigned char S[3];
+   int i;
 
if (*SIG > 45) {
if (CNTM_set(state, 2, 1, 2) < 0) {
dprintk(verbose, MB86A16_ERROR, 1, "CNTM set Error");
return -1;
}
-   wait_sym = 4;
} else {
if (CNTM_set(state, 3, 1, 2) < 0) {
dprintk(verbose, MB86A16_ERROR, 1, "CNTM set Error");
return -1;
}
-   wait_sym = 8;
}
for (i = 0; i < 3; i++) {
if (i == 0)
@@ -447,22 +441,17 @@ static int signal_det(struct mb86a16_state *state,
smrt_info_get(state, smrtd);
smrt_set(state, smrtd);
srst(state);
-   wait_t = (wait_sym + 99 * smrtd / 100) / smrtd;
-   if (wait_t == 0)
-   wait_t = 1;
msleep_interruptible(10);
if (mb86a16_read(state, 0x37, &(S[i])) != 2) {
dprintk(verbose, MB86A16_ERROR, 1, "I2C transfer 
error");
return -EREMOTEIO;
}
}
-   if ((S[1] > S[0] * 112 / 100) &&
-   (S[1] > S[2] * 112 / 100)) {
-
+   if ((S[1] > S[0] * 112 / 100) && (S[1] > S[2] * 112 / 100))
ret = 1;
-   } else {
+   else
ret = 0;
-   }
+
*SIG = S[1];
 
if (CNTM_set(state, 0, 1, 2) < 0) {
-- 
2.5.0

[media-dvb-usb-v2] question about value overwrite

[Gustavo A. R. Silva]

Hello everybody,

While looking into Coverity ID 1226934 I ran into the following piece  
of code at drivers/media/usb/dvb-usb-v2/lmedm04.c:205


205static int lme2510_stream_restart(struct dvb_usb_device *d)
206{
207struct lme2510_state *st = d->priv;
208u8 all_pids[] = LME_ALL_PIDS;
209u8 stream_on[] = LME_ST_ON_W;
210int ret;
211u8 rbuff[1];
212if (st->pid_off)
213ret = lme2510_usb_talk(d, all_pids, sizeof(all_pids),
214rbuff, sizeof(rbuff));
215/*Restart Stream Command*/
216ret = lme2510_usb_talk(d, stream_on, sizeof(stream_on),
217rbuff, sizeof(rbuff));
218return ret;
219}

The issue is that the value store in variable _ret_ at line 213 is  
overwritten by the one stored at line 216, before it can be used.


My question is if an _else_ statement is missing, or the variable  
assignment at line 213 should be removed, leaving just the call  
lme2510_usb_talk(d, all_pids, sizeof(all_pids), rbuff, sizeof(rbuff));  
in place.


Maybe either of the following patches could be applied:

index 924adfd..d573144 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -207,15 +207,15 @@ static int lme2510_stream_restart(struct  
dvb_usb_device *d)

struct lme2510_state *st = d->priv;
u8 all_pids[] = LME_ALL_PIDS;
u8 stream_on[] = LME_ST_ON_W;
-   int ret;
u8 rbuff[1];
+
if (st->pid_off)
-   ret = lme2510_usb_talk(d, all_pids, sizeof(all_pids),
+   lme2510_usb_talk(d, all_pids, sizeof(all_pids),
rbuff, sizeof(rbuff));
+
/*Restart Stream Command*/
-   ret = lme2510_usb_talk(d, stream_on, sizeof(stream_on),
+   return lme2510_usb_talk(d, stream_on, sizeof(stream_on),
rbuff, sizeof(rbuff));
-   return ret;
 }

index 924adfd..dd51f05 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -207,15 +207,15 @@ static int lme2510_stream_restart(struct  
dvb_usb_device *d)

struct lme2510_state *st = d->priv;
u8 all_pids[] = LME_ALL_PIDS;
u8 stream_on[] = LME_ST_ON_W;
-   int ret;
u8 rbuff[1];
+
if (st->pid_off)
-   ret = lme2510_usb_talk(d, all_pids, sizeof(all_pids),
+   return lme2510_usb_talk(d, all_pids, sizeof(all_pids),
rbuff, sizeof(rbuff));
-   /*Restart Stream Command*/
-   ret = lme2510_usb_talk(d, stream_on, sizeof(stream_on),
+   else
+   /*Restart Stream Command*/
+   return lme2510_usb_talk(d, stream_on, sizeof(stream_on),
rbuff, sizeof(rbuff));
-   return ret;
 }

What do you think?

I'd really appreciate any comment on this.

Thank you!
--
Gustavo A. R. Silva

[media-pci-cx25821] question about value overwrite

[Gustavo A. R. Silva]

Hello everybody,

While looking into Coverity ID 1226903 I ran into the following piece  
of code at drivers/media/pci/cx25821/cx25821-medusa-video.c:393:


393int medusa_set_videostandard(struct cx25821_dev *dev)
394{
395int status = 0;
396u32 value = 0, tmp = 0;
397
398if (dev->tvnorm & V4L2_STD_PAL_BG || dev->tvnorm & V4L2_STD_PAL_DK)
399status = medusa_initialize_pal(dev);
400else
401status = medusa_initialize_ntsc(dev);
402
403/* Enable DENC_A output */
404value = cx25821_i2c_read(>i2c_bus[0], DENC_A_REG_4, );
405value = setBitAtPos(value, 4);
406status = cx25821_i2c_write(>i2c_bus[0], DENC_A_REG_4, value);
407
408/* Enable DENC_B output */
409value = cx25821_i2c_read(>i2c_bus[0], DENC_B_REG_4, );
410value = setBitAtPos(value, 4);
411status = cx25821_i2c_write(>i2c_bus[0], DENC_B_REG_4, value);
412
413return status;
414}

The issue is that the value stored in variable _status_ at lines 399  
and 401 is overwritten by the one stored at line 406 and then at line  
411, before it can be used.


My question is if the original intention was to ORed the return  
values, something like in the following patch:


index 0a9db05..226d14f 100644
--- a/drivers/media/pci/cx25821/cx25821-medusa-video.c
+++ b/drivers/media/pci/cx25821/cx25821-medusa-video.c
@@ -403,12 +403,12 @@ int medusa_set_videostandard(struct cx25821_dev *dev)
/* Enable DENC_A output */
value = cx25821_i2c_read(>i2c_bus[0], DENC_A_REG_4, );
value = setBitAtPos(value, 4);
-   status = cx25821_i2c_write(>i2c_bus[0], DENC_A_REG_4, value);
+   status |= cx25821_i2c_write(>i2c_bus[0], DENC_A_REG_4, value);

/* Enable DENC_B output */
value = cx25821_i2c_read(>i2c_bus[0], DENC_B_REG_4, );
value = setBitAtPos(value, 4);
-   status = cx25821_i2c_write(>i2c_bus[0], DENC_B_REG_4, value);
+   status |= cx25821_i2c_write(>i2c_bus[0], DENC_B_REG_4, value);

return status;
 }

What do you think?

I'd really appreciate any comment on this.

Thank you!
--
Gustavo A. R. Silva

Re: [media-dvb-usb-v2] question about value overwrite

[Gustavo A. R. Silva]

Hi Malcolm,

Quoting Malcolm Priestley <tvbox...@gmail.com>:


Hi

On 18/05/17 20:09, Gustavo A. R. Silva wrote:


Hello everybody,

While looking into Coverity ID 1226934 I ran into the following  
piece of code at drivers/media/usb/dvb-usb-v2/lmedm04.c:205


205static int lme2510_stream_restart(struct dvb_usb_device *d)
206{
207struct lme2510_state *st = d->priv;
208u8 all_pids[] = LME_ALL_PIDS;
209u8 stream_on[] = LME_ST_ON_W;
210int ret;
211u8 rbuff[1];
212if (st->pid_off)
213ret = lme2510_usb_talk(d, all_pids, sizeof(all_pids),
214rbuff, sizeof(rbuff));
215/*Restart Stream Command*/
216ret = lme2510_usb_talk(d, stream_on, sizeof(stream_on),
217rbuff, sizeof(rbuff));
218return ret;
219}


It is a mistake it should have been ORed ad in |= as  
lme2510_usb_talk only returns three states.




I see now. The idea is to code something similar to the following  
piece of code in the same file:


242
243ret |= lme2510_usb_talk(d, pid_buff ,
244sizeof(pid_buff) , rbuf, sizeof(rbuf));
245
246if (st->stream_on)
247ret |= lme2510_stream_restart(d);
248
249return ret;

right?

So in this case, the following patch would properly fix the bug:

index 924adfd..3ab1754 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -207,13 +207,14 @@ static int lme2510_stream_restart(struct  
dvb_usb_device *d)

struct lme2510_state *st = d->priv;
u8 all_pids[] = LME_ALL_PIDS;
u8 stream_on[] = LME_ST_ON_W;
-   int ret;
+   int ret = 0;
u8 rbuff[1];
+
if (st->pid_off)
ret = lme2510_usb_talk(d, all_pids, sizeof(all_pids),
rbuff, sizeof(rbuff));
/*Restart Stream Command*/
-   ret = lme2510_usb_talk(d, stream_on, sizeof(stream_on),
+   ret |= lme2510_usb_talk(d, stream_on, sizeof(stream_on),
rbuff, sizeof(rbuff));
return ret;
 }

What do you think?


So if an error is in the running it will be returned to user.

The first of your patches is better and more or less the same, the  
second would break driver, restart is not an else condition.




Thank you for the clarification.
--
Gustavo A. R. Silva

[PATCH] media: platform: s3c-camif: fix arguments position in function call

[Gustavo A. R. Silva]

Hi Sylwester,

Here is another patch in case you decide that it is
better to apply this one.

Thanks
--
Gustavo A. R. Silva


==

Fix the position of the arguments in function call.

Addresses-Coverity-ID: 1248800
Addresses-Coverity-ID: 1269141
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/s3c-camif/camif-capture.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
b/drivers/media/platform/s3c-camif/camif-capture.c
index 1b30be72..25c7a7d 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -80,7 +80,7 @@ static int s3c_camif_hw_init(struct camif_dev *camif, struct 
camif_vp *vp)
camif_hw_set_test_pattern(camif, camif->test_pattern);
if (variant->has_img_effect)
camif_hw_set_effect(camif, camif->colorfx,
-   camif->colorfx_cb, camif->colorfx_cr);
+   camif->colorfx_cr, camif->colorfx_cb);
if (variant->ip_revision == S3C6410_CAMIF_IP_REV)
camif_hw_set_input_path(vp);
camif_cfg_video_path(vp);
@@ -364,7 +364,7 @@ irqreturn_t s3c_camif_irq_handler(int irq, void *priv)
camif_hw_set_test_pattern(camif, camif->test_pattern);
if (camif->variant->has_img_effect)
camif_hw_set_effect(camif, camif->colorfx,
-   camif->colorfx_cb, camif->colorfx_cr);
+   camif->colorfx_cr, camif->colorfx_cb);
vp->state &= ~ST_VP_CONFIG;
}
 unlock:
-- 
2.5.0

Re: [PATCH] media: platform: s3c-camif: fix arguments position in function call

[Gustavo A. R. Silva]

Hi Sylwester,

Quoting Sylwester Nawrocki <s.nawro...@samsung.com>:


On 06/02/2017 05:43 AM, Gustavo A. R. Silva wrote:

Hi Sylwester,

Here is another patch in case you decide that it is
better to apply this one.


Thanks, I applied this patch.  In future please put any comments only after
the scissors ("---") line, the comments can be then discarded automatically
and there will be no need for manually editing the patch before applying.



OK, I will do that.


--
Regards,
Sylwester


Fix the position of the arguments in function call.

Addresses-Coverity-ID: 1248800
Addresses-Coverity-ID: 1269141
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---

^



I got it.

Thank you
--
Gustavo A. R. Silva

[media-af9013] question about return value in function af9013_wr_regs()

[Gustavo A. R. Silva]

ite multiple registers */
-static int af9013_wr_regs(struct af9013_state *priv, u16 reg, const u8 *val,
+static void af9013_wr_regs(struct af9013_state *priv, u16 reg, const u8 *val,
int len)
 {
-   int ret, i;
+   int i;
u8 mbox = (0 << 7)|(0 << 6)|(1 << 1)|(1 << 0);

if ((priv->config.ts_mode == AF9013_TS_USB) &&
((reg & 0xff00) != 0xff00) && ((reg & 0xff00) != 0xae00)) {
mbox |= ((len - 1) << 2);
-   ret = af9013_wr_regs_i2c(priv, mbox, reg, val, len);
+   af9013_wr_regs_i2c(priv, mbox, reg, val, len);
} else {
for (i = 0; i < len; i++) {
-   ret = af9013_wr_regs_i2c(priv, mbox, reg+i, val+i, 1);
-   if (ret)
-   goto err;
+   af9013_wr_regs_i2c(priv, mbox, reg+i, val+i, 1);
}
}
-
-err:
-   return 0;
 }

and of course, all of its callers also should be refactored  
accordinly.  Otherwise, the value contained in variable _ret_ should  
be properly returned.


I can do the refactoring but first it would be great to hear your  
opinions on this.


I'd really appreciate any comment on this.

Thank you!
--
Gustavo A. R. Silva

[media-m5602-po1030] question about return value check

[Gustavo A. R. Silva]

t lines  
312 and 340 is not being evaluated as it happens in other instances  
after calling m5602_write_sensor().


I'm suspicious this is not the original intention and maybe a patch  
like the following could be applied:


--- a/drivers/media/usb/gspca/m5602/m5602_po1030.c+++  
b/drivers/media/usb/gspca/m5602/m5602_po1030.c@@ -310,6 +310,8 @@ int  
po1030_start(struct sd *sd)


data = (height + 1) & 0xff;
err = m5602_write_sensor(sd, PO1030_WINDOWHEIGHT_L, , 1);
+   if (err < 0)
+   return err;

height += 6;
width -= 1;
@@ -338,6 +340,8 @@ int po1030_start(struct sd *sd)

data = (height + 3) & 0xff;
err = m5602_write_sensor(sd, PO1030_WINDOWHEIGHT_L, , 1);
+   if (err < 0)
+   return err;

height += 12;
width -= 2;


What do you think?

It would be great to hear your opinions about it.

I'd really appreciate any comment on this.

Thank you!
--
Gustavo A. R. Silva

[PATCH] af9013: add check on af9013_wr_regs() return value

[Gustavo A. R. Silva]

Check return value from call to af9013_wr_regs(), so in case of
error print debug message and return.

Addresses-Coverity-ID: 1227035
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/dvb-frontends/af9013.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/dvb-frontends/af9013.c 
b/drivers/media/dvb-frontends/af9013.c
index 51f942f..dc7ccda 100644
--- a/drivers/media/dvb-frontends/af9013.c
+++ b/drivers/media/dvb-frontends/af9013.c
@@ -608,6 +608,8 @@ static int af9013_set_frontend(struct dvb_frontend *fe)
 
ret = af9013_wr_regs(state, 0xae00, coeff_lut[i].val,
sizeof(coeff_lut[i].val));
+   if (ret)
+   goto err;
}
 
/* program frequency control */
-- 
2.5.0

Re: [git:media_tree/master] [media] s3c-camif: fix arguments position in a function call

[Gustavo A. R. Silva]

Hi Sylwester,

Quoting Mauro Carvalho Chehab <mche...@s-opensource.com>:

This is an automatic generated email to let you know that the  
following patch were queued:


Subject: [media] s3c-camif: fix arguments position in a function call
Author:  Gustavo A. R. Silva <garsi...@embeddedor.com>
Date:Fri Jun 2 00:43:41 2017 -0300

Fix the position of arguments so camif->colorfx_cb, camif->colorfx_cr
are passed in proper order to the camif_hw_set_effect() function.

Addresses-Coverity-ID: 1248800
Addresses-Coverity-ID: 1269141

[s.nawro...@samsung.com: edited commit message ]


Thank you for your editing of the commit message. I will elaborate  
further in future patches.


Best regards.
--
Gustavo A. R. Silva

[PATCH] i2c: tc358743: remove useless variable assignment in tc358743_isr

[Gustavo A. R. Silva]

Remove useless variable assignment in function tc358743_isr().

The value stored in variable _intstatus_ at line 1299 is
overwritten at line 1302, just before it can be used.

Addresses-Coverity-ID: 1397678
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/i2c/tc358743.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 3251cba..e8b23f3 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1296,7 +1296,6 @@ static int tc358743_isr(struct v4l2_subdev *sd, u32 
status, bool *handled)
tc358743_csi_err_int_handler(sd, handled);
 
i2c_wr16(sd, INTSTATUS, MASK_CSI_INT);
-   intstatus &= ~MASK_CSI_INT;
}
 
intstatus = i2c_rd16(sd, INTSTATUS);
-- 
2.5.0

[PATCH] media: platform: s3c-camif: fix function prototype

[Gustavo A. R. Silva]

Fix function prototype so the position of arguments camif->colorfx_cb and
camif->colorfx_cr match the order of the parameters when calling
camif_hw_set_effect() function.

Addresses-Coverity-ID: 1248800
Addresses-Coverity-ID: 1269141
Cc: Sylwester Nawrocki <sylvester.nawro...@gmail.com>
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/s3c-camif/camif-regs.c | 2 +-
 drivers/media/platform/s3c-camif/camif-regs.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/s3c-camif/camif-regs.c 
b/drivers/media/platform/s3c-camif/camif-regs.c
index 812fb3a..d70ffef 100644
--- a/drivers/media/platform/s3c-camif/camif-regs.c
+++ b/drivers/media/platform/s3c-camif/camif-regs.c
@@ -58,7 +58,7 @@ void camif_hw_set_test_pattern(struct camif_dev *camif, 
unsigned int pattern)
 }
 
 void camif_hw_set_effect(struct camif_dev *camif, unsigned int effect,
-   unsigned int cr, unsigned int cb)
+   unsigned int cb, unsigned int cr)
 {
static const struct v4l2_control colorfx[] = {
{ V4L2_COLORFX_NONE,CIIMGEFF_FIN_BYPASS },
diff --git a/drivers/media/platform/s3c-camif/camif-regs.h 
b/drivers/media/platform/s3c-camif/camif-regs.h
index 5ad36c1..dfb49a5 100644
--- a/drivers/media/platform/s3c-camif/camif-regs.h
+++ b/drivers/media/platform/s3c-camif/camif-regs.h
@@ -255,7 +255,7 @@ void camif_hw_set_output_dma(struct camif_vp *vp);
 void camif_hw_set_target_format(struct camif_vp *vp);
 void camif_hw_set_test_pattern(struct camif_dev *camif, unsigned int pattern);
 void camif_hw_set_effect(struct camif_dev *camif, unsigned int effect,
-   unsigned int cr, unsigned int cb);
+   unsigned int cb, unsigned int cr);
 void camif_hw_set_output_addr(struct camif_vp *vp, struct camif_addr *paddr,
  int index);
 void camif_hw_dump_regs(struct camif_dev *camif, const char *label);
-- 
2.5.0

Re: [media-s3c-camif] question about arguments position

[Gustavo A. R. Silva]

Hello Sylwester,

Quoting Sylwester Nawrocki <sylvester.nawro...@gmail.com>:


Hi Gustavo,

On 05/04/2017 09:05 PM, Gustavo A. R. Silva wrote:

The issue here is that the position of arguments in the call to
camif_hw_set_effect() function do not match the order of the parameters:

camif->colorfx_cb is passed to cr
camif->colorfx_cr is passed to cb

This is the function prototype:

void camif_hw_set_effect(struct camif_dev *camif, unsigned int effect,
unsigned int cr, unsigned int cb)

My question here is if this is intentional?

In case it is not, I will send a patch to fix it. But first it would be
great to hear any comment about it.


You are right, it seems you have found a real bug. Feel free to send a patch.
The best thing to do now might be to change the function prototype to:

void camif_hw_set_effect(struct camif_dev *camif, unsigned int effect,
 unsigned int cb, unsigned int cr)



OK, I'll send a patch for this shortly.

Thanks for clarifying.
--
Gustavo A. R. Silva

[media-s3c-camif] question about arguments position

[Gustavo A. R. Silva]

Hello everybody,

While looking into Coverity ID 1248800 I ran into the following piece  
of code at drivers/media/platform/s3c-camif/camif-capture.c:67:


/* Locking: called with camif->slock spinlock held */
static int s3c_camif_hw_init(struct camif_dev *camif, struct camif_vp *vp)
{
const struct s3c_camif_variant *variant = camif->variant;

if (camif->sensor.sd == NULL || vp->out_fmt == NULL)
return -EINVAL;

if (variant->ip_revision == S3C244X_CAMIF_IP_REV)
camif_hw_clear_fifo_overflow(vp);
camif_hw_set_camera_bus(camif);
camif_hw_set_source_format(camif);
camif_hw_set_camera_crop(camif);
camif_hw_set_test_pattern(camif, camif->test_pattern);
if (variant->has_img_effect)
camif_hw_set_effect(camif, camif->colorfx,
camif->colorfx_cb, camif->colorfx_cr);
if (variant->ip_revision == S3C6410_CAMIF_IP_REV)
camif_hw_set_input_path(vp);
camif_cfg_video_path(vp);
vp->state &= ~ST_VP_CONFIG;

return 0;
}


The issue here is that the position of arguments in the call to  
camif_hw_set_effect() function do not match the order of the parameters:


camif->colorfx_cb is passed to cr
camif->colorfx_cr is passed to cb

This is the function prototype:

void camif_hw_set_effect(struct camif_dev *camif, unsigned int effect,
unsigned int cr, unsigned int cb)

My question here is if this is intentional?

In case it is not, I will send a patch to fix it. But first it would  
be great to hear any comment about it.


By the way... the same is happening at  
drivers/media/platform/s3c-camif/camif-capture.c:366


Thank you
--
Gustavo A. R. Silva

[PATCH] ov9655: fix potential integer overflow

[Gustavo A. R. Silva]

Add suffix ULL to constant 1 rather than casting the result of the
operation in order to avoid a potential integer overflow. This constant
is used in a context that expects an expression of type u64.

Addresses-Coverity-ID: 1324146
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/i2c/ov9650.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 6ffb460..91a09ee 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -1129,8 +1129,8 @@ static int __ov965x_set_frame_interval(struct ov965x 
*ov965x,
if (fi->interval.denominator == 0)
return -EINVAL;
 
-   req_int = (u64)(fi->interval.numerator * 1) /
-   fi->interval.denominator;
+   req_int = (fi->interval.numerator * 1ULL) /
+ fi->interval.denominator;
 
for (i = 0; i < ARRAY_SIZE(ov965x_intervals); i++) {
const struct ov965x_interval *iv = _intervals[i];
-- 
2.7.4

[PATCH] siano: fix a potential integer overflow

[Gustavo A. R. Silva]

Add suffix ULL to constant 65535 in order to avoid a potential
integer overflow. This constant is used in a context that
expects an expression of type u64.

Addresses-Coverity-ID: 1056806
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/common/siano/smsdvb-main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/common/siano/smsdvb-main.c 
b/drivers/media/common/siano/smsdvb-main.c
index affde14..166428c 100644
--- a/drivers/media/common/siano/smsdvb-main.c
+++ b/drivers/media/common/siano/smsdvb-main.c
@@ -271,7 +271,7 @@ static void smsdvb_update_per_slices(struct smsdvb_client_t 
*client,
c->post_bit_count.stat[0].uvalue += p->ber_bit_count;
 
/* Legacy PER/BER */
-   tmp = p->ets_packets * 65535;
+   tmp = p->ets_packets * 65535ULL;
if (p->ts_packets + p->ets_packets)
do_div(tmp, p->ts_packets + p->ets_packets);
client->legacy_per = tmp;
-- 
2.7.4

Re: [PATCH] media: venus: fix duplicated code for different branches

[Gustavo A. R. Silva]

Hi Stanimir,

On 08/18/2017 02:52 AM, Stanimir Varbanov wrote:

Hi Gustavo,

On 08/18/2017 02:12 AM, Gustavo A. R. Silva wrote:

Refactor code in order to avoid identical code for different branches.

This issue was detected with the help of Coccinelle.

Addresses-Coverity-ID: 1415317
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
This code was reported by Coverity and it was tested by compilation only.
Please, verify if this is an actual bug.


Yes looks like copy/paste error, and yes it is a bug.



Thank you for reviewing it.



 drivers/media/platform/qcom/venus/helpers.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/helpers.c 
b/drivers/media/platform/qcom/venus/helpers.c
index 5f4434c..8a5c467 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -240,11 +240,7 @@ static void return_buf_error(struct venus_inst *inst,
 {
struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;

-   if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
-   v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
-   else
-   v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);


the correct fix must replace the second v4l2_m2m_src_* with v4l2_m2m_dst_*.



I'll send a patch to fix this bug shortly


-
+   v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }






Thanks!
--
Gustavo A. R. Silva

[PATCH v2] venus: fix copy/paste error in return_buf_error

[Gustavo A. R. Silva]

Call function v4l2_m2m_dst_buf_remove_by_buf() instead of
v4l2_m2m_src_buf_remove_by_buf()

Addresses-Coverity-ID: 1415317
Cc: Stanimir Varbanov <stanimir.varba...@linaro.org>
Cc: Hans Verkuil <hverk...@xs4all.nl>
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
Changes in v2:
 Stanimir Varbanov confirmed this is a bug. The correct fix is to call
 function v4l2_m2m_dst_buf_remove_by_buf instead of function
 v4l2_m2m_src_buf_remove_by_buf in the _else_ branch.

 drivers/media/platform/qcom/venus/helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/helpers.c 
b/drivers/media/platform/qcom/venus/helpers.c
index 5f4434c..2d61879 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -243,7 +243,7 @@ static void return_buf_error(struct venus_inst *inst,
if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
else
-   v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
+   v4l2_m2m_dst_buf_remove_by_buf(m2m_ctx, vbuf);
 
v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }
-- 
2.5.0

[PATCH] staging: media: imx: fix inconsistent IS_ERR and PTR_ERR

[Gustavo A. R. Silva]

Fix inconsistent IS_ERR and PTR_ERR in csi_link_validate.
The proper pointer to be passed as argument is sensor.

This issue was detected with the help of Coccinelle.

Reported-by: Julia Lawall <julia.law...@lip6.fr>
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
This code was tested by compilation only.

 drivers/staging/media/imx/imx-media-csi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 6d85611..2fa72c1 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -989,7 +989,7 @@ static int csi_link_validate(struct v4l2_subdev *sd,
sensor = __imx_media_find_sensor(priv->md, >sd.entity);
if (IS_ERR(sensor)) {
v4l2_err(>sd, "no sensor attached\n");
-   return PTR_ERR(priv->sensor);
+   return PTR_ERR(sensor);
}
 
mutex_lock(>lock);
-- 
2.7.4

[PATCH] davinci: vpif_capture: add NULL check on devm_kzalloc return value

[Gustavo A. R. Silva]

Check return value from call to devm_kzalloc() in order to prevent
a NULL pointer dereference.

This issue was detected with the help of Coccinelle.

Fixes: 4a5f8ae50b66 ("[media] davinci: vpif_capture: get subdevs from DT when 
available")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index a89367a..3879c7c 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1550,6 +1550,8 @@ vpif_capture_get_pdata(struct platform_device *pdev)
sizeof(*chan->inputs) *
VPIF_CAPTURE_NUM_CHANNELS,
GFP_KERNEL);
+   if (!chan->inputs)
+   return NULL;
 
chan->input_count++;
chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
-- 
2.7.4

Re: [PATCH] au0828: fix use-after-free at USB probing

[Gustavo A. R. Silva]

Hi Andrey,

I have successfully installed and tested syzkaller with QEMU. Can you  
please tell me how to reproduce this bug or share with me the full  
crash report?


Also, can you point me out to the PoC file?

Much appreciated
Thank you!
--
Gustavo A. R. Silva

Quoting Andrey Konovalov <andreyk...@google.com>:


On Fri, Nov 10, 2017 at 6:35 PM, Gustavo A. R. Silva
<garsi...@embeddedor.com> wrote:


Quoting Andrey Konovalov <andreyk...@google.com>:


On Fri, Nov 10, 2017 at 1:21 AM, Gustavo A. R. Silva
<garsi...@embeddedor.com> wrote:


Hi Andrey,

Could you please try this patch?

Thank you


Hi!

Sorry for the delay.

With this patch I still see the same report:

au0828: recv_control_msg() Failed receiving control message, error -71.
au0828: recv_control_msg() Failed receiving control message, error -71.
au0828: recv_control_msg() Failed receiving control message, error -71.
au8522_writereg: writereg error (reg == 0x106, val == 0x0001, ret == -5)
usb 1-1: selecting invalid altsetting 5
au0828: Failure setting usb interface0 to as5
au0828: au0828_usb_probe() au0282_dev_register failed to register on V4L2
au0828: probe of 1-1:0.0 failed with error -22
usb 1-1: USB disconnect, device number 3
==
BUG: KASAN: use-after-free in __list_del_entry_valid+0xda/0xf3
Read of size 8 at addr 880062a74410 by task kworker/0:1/24

CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted
4.14.0-rc8-44455-ge2105594a876-dirty #111
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
 __dump_stack lib/dump_stack.c:17
 dump_stack+0xe1/0x157 lib/dump_stack.c:53
 print_address_description+0x71/0x234 mm/kasan/report.c:252
 kasan_report_error mm/kasan/report.c:351
 kasan_report+0x173/0x270 mm/kasan/report.c:409
 __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430
 __list_del_entry_valid+0xda/0xf3 lib/list_debug.c:54
 __list_del_entry ./include/linux/list.h:117
 list_del_init ./include/linux/list.h:159
 device_pm_remove+0x4a/0x1e7 drivers/base/power/main.c:149
 device_del+0x599/0xa70 drivers/base/core.c:1986
 usb_disable_device+0x223/0x710 drivers/usb/core/message.c:1170
 usb_disconnect+0x285/0x7f0 drivers/usb/core/hub.c:2205
 hub_port_connect drivers/usb/core/hub.c:4838
 hub_port_connect_change drivers/usb/core/hub.c:5093
 port_event drivers/usb/core/hub.c:5199
 hub_event_impl+0x10ec/0x3440 drivers/usb/core/hub.c:5311
 hub_event+0x38/0x50 drivers/usb/core/hub.c:5209
 process_one_work+0x925/0x15d0 kernel/workqueue.c:2113
 process_scheduled_works kernel/workqueue.c:2173
 worker_thread+0x72e/0x10d0 kernel/workqueue.c:2249
 kthread+0x346/0x410 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:432

The buggy address belongs to the page:
page:ea00018a9d00 count:0 mapcount:-127 mapping:  (null)  
index:0x0

flags: 0x100()
raw: 0100   ff80
raw: 88007fffa690 ea00018e6120 0002 
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 880062a74300: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 880062a74380: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

880062a74400: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

 ^
 880062a74480: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 880062a74500: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
=

Thanks!




Hi Gustavo,

With your patch I get a different crash. Not sure if it's another bug
or the same one manifesting differently.



That's the same one. It seems that the best solution is to remove the kfree
after the mutex_unlock and let the device resources be freed in
au0828_usb_disconnect.

Please try the following patch instead.

I appreciate your help.

Thank you, Andrey.

---
 drivers/media/usb/au0828/au0828-core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/usb/au0828/au0828-core.c
b/drivers/media/usb/au0828/au0828-core.c
index cd363a2..257ae0d 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -629,7 +629,6 @@ static int au0828_usb_probe(struct usb_interface
*interface,
pr_err("%s() au0282_dev_register failed to register on
V4L2\n",
__func__);
mutex_unlock(>lock);
-   kfree(dev);
goto done;
}

--
2.7.4



au0828: recv_control_msg() Failed receiving control message, error -71.
au0828: recv_control_msg() Failed receiving control message, error -71.
au8522_writereg: writereg error (reg == 0x106, val == 0x0001, ret == -5)
usb 1-1: selecting invalid altsetting 5
au0828: Failure setting usb interface0 to as5
au0828: au0828_usb_probe() au0282_dev_register failed to register on

Re: [PATCH] c8sectpfe: fix potential NULL pointer dereference in c8sectpfe_timer_interrupt

[Gustavo A. R. Silva]

On 11/21/2017 02:22 AM, Patrice CHOTARD wrote:

Hi Gustavo

On 11/20/2017 03:00 PM, Gustavo A. R. Silva wrote:

_channel_ is being dereferenced before it is null checked, hence there is a
potential null pointer dereference. Fix this by moving the pointer dereference
after _channel_ has been null checked.

This issue was detected with the help of Coccinelle.

Fixes: c5f5d0f99794 ("[media] c8sectpfe: STiH407/10 Linux DVB demux support")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
   drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c 
b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index 59280ac..23d0ced 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -83,7 +83,7 @@ static void c8sectpfe_timer_interrupt(unsigned long 
ac8sectpfei)
   static void channel_swdemux_tsklet(unsigned long data)
   {
struct channel_info *channel = (struct channel_info *)data;
-   struct c8sectpfei *fei = channel->fei;
+   struct c8sectpfei *fei;
unsigned long wp, rp;
int pos, num_packets, n, size;
u8 *buf;
@@ -91,6 +91,8 @@ static void channel_swdemux_tsklet(unsigned long data)
if (unlikely(!channel || !channel->irec))
return;
   
+	fei = channel->fei;

+
wp = readl(channel->irec + DMA_PRDS_BUSWP_TP(0));
rp = readl(channel->irec + DMA_PRDS_BUSRP_TP(0));
   


Acked-by: Patrice Chotard <patrice.chot...@st.com>

Thanks


Thank you, Patrice.

--
Gustavo A. R. Silva

Re: [PATCH] au0828: fix use-after-free at USB probing

[Gustavo A. R. Silva]

Hey Andrey,

Quoting Andrey Konovalov <andreyk...@google.com>:


On Thu, Nov 23, 2017 at 2:31 AM, Gustavo A. R. Silva
<garsi...@embeddedor.com> wrote:

Hi Andrey,

I have successfully installed and tested syzkaller with QEMU. Can you please
tell me how to reproduce this bug or share with me the full crash report?

Also, can you point me out to the PoC file?


Hi Gustavo,

Sorry for the delay.



No worries.


I've now published the USB fuzzing prototype, so here's how you can
reproduce this:

1. Get Linux 4.15-rc3 upstream kernel
(50c4c4e268a2d7a3e58ebb698ac74da0de40ae36).

2. Apply this patch (it adds a new interface to emulate USB devices):
https://github.com/google/syzkaller/blob/usb-fuzzer/tools/usb/0002-usb-fuzzer-main-usb-gadget-fuzzer-driver.patch

3. Build the kernel with the attached .config (you need relatively new
GCC to make KASAN work).

4. Run the attached reproducer.

Also attaching the full kernel log.



Awesome. :D I'll try this.

Thank you!
--
Gustavo A. R. Silva

Re: [PATCH] au0828: fix use-after-free at USB probing

[Gustavo A. R. Silva]

Quoting Andrey Konovalov <andreyk...@google.com>:


On Fri, Nov 10, 2017 at 1:21 AM, Gustavo A. R. Silva
<garsi...@embeddedor.com> wrote:

Hi Andrey,

Could you please try this patch?

Thank you


Hi Gustavo,

With your patch I get a different crash. Not sure if it's another bug
or the same one manifesting differently.



That's the same one. It seems that the best solution is to remove the  
kfree after the mutex_unlock and let the device resources be freed in  
au0828_usb_disconnect.


Please try the following patch instead.

I appreciate your help.

Thank you, Andrey.

---
 drivers/media/usb/au0828/au0828-core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/usb/au0828/au0828-core.c  
b/drivers/media/usb/au0828/au0828-core.c

index cd363a2..257ae0d 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -629,7 +629,6 @@ static int au0828_usb_probe(struct usb_interface  
*interface,
pr_err("%s() au0282_dev_register failed to register  
on V4L2\n",

__func__);
mutex_unlock(>lock);
-   kfree(dev);
goto done;
}

--
2.7.4


au0828: recv_control_msg() Failed receiving control message, error -71.
au0828: recv_control_msg() Failed receiving control message, error -71.
au8522_writereg: writereg error (reg == 0x106, val == 0x0001, ret == -5)
usb 1-1: selecting invalid altsetting 5
au0828: Failure setting usb interface0 to as5
au0828: au0828_usb_probe() au0282_dev_register failed to register on V4L2
au0828: probe of 1-1:0.0 failed with error -22
usb 1-1: USB disconnect, device number 2
==
BUG: KASAN: use-after-free in __list_del_entry_valid+0xda/0xf3
Read of size 8 at addr 8800641d0410 by task kworker/0:1/24

CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted
4.14.0-rc5-43687-g72e555fa3d2e-dirty #105
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
 __dump_stack lib/dump_stack.c:16
 dump_stack+0xc1/0x11f lib/dump_stack.c:52
 print_address_description+0x71/0x234 mm/kasan/report.c:252
 kasan_report_error mm/kasan/report.c:351
 kasan_report+0x173/0x270 mm/kasan/report.c:409
 __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430
 __list_del_entry_valid+0xda/0xf3 lib/list_debug.c:54
 __list_del_entry ./include/linux/list.h:116
 list_del_init ./include/linux/list.h:158
 device_pm_remove+0x4a/0x1da drivers/base/power/main.c:149
 device_del+0x55f/0xa30 drivers/base/core.c:1986
 usb_disable_device+0x1df/0x670 drivers/usb/core/message.c:1170
 usb_disconnect+0x260/0x7a0 drivers/usb/core/hub.c:2124
 hub_port_connect drivers/usb/core/hub.c:4754
 hub_port_connect_change drivers/usb/core/hub.c:5009
 port_event drivers/usb/core/hub.c:5115
 hub_event+0xe09/0x2eb0 drivers/usb/core/hub.c:5195
 process_one_work+0x86d/0x13e0 kernel/workqueue.c:2119
 process_scheduled_works kernel/workqueue.c:2179
 worker_thread+0x689/0xea0 kernel/workqueue.c:2255
 kthread+0x334/0x400 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431

The buggy address belongs to the page:
page:ea0001907400 count:0 mapcount:-127 mapping:  (null)  
index:0x0

flags: 0x100()
raw: 0100   ff80
raw: ea00018a8f20 88007fffa690 0002 
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 8800641d0300: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 8800641d0380: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

8800641d0400: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

 ^
 8800641d0480: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 8800641d0500: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==

Thanks!




The device is typically freed on failure after trying to set
USB interface0 to as5 in function au0828_analog_register.

Fix use-after-free by returning the error value inmediately
after failure, instead of jumping to au0828_usb_disconnect
where _dev_ is also freed.

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/usb/au0828/au0828-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-core.c  
b/drivers/media/usb/au0828/au0828-core.c

index cd363a2..b4abd90 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -630,7 +630,7 @@ static int au0828_usb_probe(struct  
usb_interface *interface,

__func__);
mutex_unlock(>lock);
kfree(dev);
-   goto done;
+   return retval;
}

/* Digital TV */
@@ -655,7 +655,6 @@ static int au0828_usb_probe

[PATCH] usb: uvc_debugfs: remove unnecessary NULL check before debugfs_remove_recursive

[Gustavo A. R. Silva]

NULL check before freeing functions like debugfs_remove_recursive
is not needed.

This issue was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/usb/uvc/uvc_debugfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_debugfs.c 
b/drivers/media/usb/uvc/uvc_debugfs.c
index 368f8f8..6995aeb 100644
--- a/drivers/media/usb/uvc/uvc_debugfs.c
+++ b/drivers/media/usb/uvc/uvc_debugfs.c
@@ -128,6 +128,5 @@ void uvc_debugfs_init(void)
 
 void uvc_debugfs_cleanup(void)
 {
-   if (uvc_debugfs_root_dir != NULL)
-   debugfs_remove_recursive(uvc_debugfs_root_dir);
+   debugfs_remove_recursive(uvc_debugfs_root_dir);
 }
-- 
2.7.4

[PATCH] c8sectpfe: fix potential NULL pointer dereference in c8sectpfe_timer_interrupt

[Gustavo A. R. Silva]

_channel_ is being dereferenced before it is null checked, hence there is a
potential null pointer dereference. Fix this by moving the pointer dereference
after _channel_ has been null checked.

This issue was detected with the help of Coccinelle.

Fixes: c5f5d0f99794 ("[media] c8sectpfe: STiH407/10 Linux DVB demux support")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c 
b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index 59280ac..23d0ced 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -83,7 +83,7 @@ static void c8sectpfe_timer_interrupt(unsigned long 
ac8sectpfei)
 static void channel_swdemux_tsklet(unsigned long data)
 {
struct channel_info *channel = (struct channel_info *)data;
-   struct c8sectpfei *fei = channel->fei;
+   struct c8sectpfei *fei;
unsigned long wp, rp;
int pos, num_packets, n, size;
u8 *buf;
@@ -91,6 +91,8 @@ static void channel_swdemux_tsklet(unsigned long data)
if (unlikely(!channel || !channel->irec))
return;
 
+   fei = channel->fei;
+
wp = readl(channel->irec + DMA_PRDS_BUSWP_TP(0));
rp = readl(channel->irec + DMA_PRDS_BUSRP_TP(0));
 
-- 
2.7.4

[PATCH] au0828: fix use-after-free at USB probing

[Gustavo A. R. Silva]

Hi Andrey,

Could you please try this patch?

Thank you


The device is typically freed on failure after trying to set
USB interface0 to as5 in function au0828_analog_register.

Fix use-after-free by returning the error value inmediately
after failure, instead of jumping to au0828_usb_disconnect
where _dev_ is also freed.

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/usb/au0828/au0828-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-core.c 
b/drivers/media/usb/au0828/au0828-core.c
index cd363a2..b4abd90 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -630,7 +630,7 @@ static int au0828_usb_probe(struct usb_interface *interface,
__func__);
mutex_unlock(>lock);
kfree(dev);
-   goto done;
+   return retval;
}
 
/* Digital TV */
@@ -655,7 +655,6 @@ static int au0828_usb_probe(struct usb_interface *interface,
 
retval = au0828_media_device_register(dev, usbdev);
 
-done:
if (retval < 0)
au0828_usb_disconnect(interface);
 
-- 
2.7.4

[PATCH] usb: dvb-usb-v2: dvb_usb_core: remove redundant code in dvb_usb_fe_sleep

[Gustavo A. R. Silva]

Check on return value and goto instruction is redundant as the code
that follows is the goto label err.

Addresses-Coverity-ID: 1268783
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c 
b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
index 096bb75..2bf3bd8 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
@@ -628,8 +628,7 @@ static int dvb_usb_fe_sleep(struct dvb_frontend *fe)
}
 
ret = dvb_usbv2_device_power_ctrl(d, 0);
-   if (ret < 0)
-   goto err;
+
 err:
if (!adap->suspend_resume_active) {
adap->active_fe = -1;
-- 
2.7.4

Re: [PATCH] c8sectpfe: fix potential NULL pointer dereference in c8sectpfe_timer_interrupt

[Gustavo A. R. Silva]

Hello,

On 11/21/2017 02:22 AM, Patrice CHOTARD wrote:

Hi Gustavo

On 11/20/2017 03:00 PM, Gustavo A. R. Silva wrote:

_channel_ is being dereferenced before it is null checked, hence there is a
potential null pointer dereference. Fix this by moving the pointer dereference
after _channel_ has been null checked.

This issue was detected with the help of Coccinelle.

Fixes: c5f5d0f99794 ("[media] c8sectpfe: STiH407/10 Linux DVB demux support")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
   drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c 
b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index 59280ac..23d0ced 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -83,7 +83,7 @@ static void c8sectpfe_timer_interrupt(unsigned long 
ac8sectpfei)
   static void channel_swdemux_tsklet(unsigned long data)
   {
struct channel_info *channel = (struct channel_info *)data;
-   struct c8sectpfei *fei = channel->fei;
+   struct c8sectpfei *fei;
unsigned long wp, rp;
int pos, num_packets, n, size;
u8 *buf;
@@ -91,6 +91,8 @@ static void channel_swdemux_tsklet(unsigned long data)
if (unlikely(!channel || !channel->irec))
return;
   
+	fei = channel->fei;

+
wp = readl(channel->irec + DMA_PRDS_BUSWP_TP(0));
rp = readl(channel->irec + DMA_PRDS_BUSRP_TP(0));
   


Acked-by: Patrice Chotard <patrice.chot...@st.com>

Thanks


Thank you, Patrice.

--
Gustavo A. R. Silva

Re: [git:media_tree/master] media: davinci: vpif_capture: add NULL check on devm_kzalloc return value

[Gustavo A. R. Silva]

Quoting Mauro Carvalho Chehab <mche...@s-opensource.com>:

This is an automatic generated email to let you know that the  
following patch were queued:


Subject: media: davinci: vpif_capture: add NULL check on  
devm_kzalloc return value

Author:  Gustavo A. R. Silva <garsi...@embeddedor.com>
Date:Wed Nov 22 22:34:44 2017 -0500

Check return value from call to devm_kzalloc() in order to prevent
a NULL pointer dereference.

This issue was detected with the help of Coccinelle.

Fixes: 4a5f8ae50b66 ("[media] davinci: vpif_capture: get subdevs  
from DT when available")


Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>



Thank you, Hans and Mauro for reviewing my patches.

I wonder if the following patch managed to resolved the use-after-free bug:

https://patchwork.linuxtv.org/patch/45391/

Thanks!
--
Gustavo A. R. Silva

[PATCH] st-hva: hva-h264: use swap macro in hva_h264_encode

[Gustavo A. R. Silva]

Make use of the swap macro and remove unnecessary variable tmp_frame.
This makes the code easier to read and maintain.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
 drivers/media/platform/sti/hva/hva-h264.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/media/platform/sti/hva/hva-h264.c 
b/drivers/media/platform/sti/hva/hva-h264.c
index e6f247a..a7e5eed 100644
--- a/drivers/media/platform/sti/hva/hva-h264.c
+++ b/drivers/media/platform/sti/hva/hva-h264.c
@@ -999,7 +999,6 @@ static int hva_h264_encode(struct hva_ctx *pctx, struct 
hva_frame *frame,
 {
struct hva_h264_ctx *ctx = (struct hva_h264_ctx *)pctx->priv;
struct hva_h264_task *task = (struct hva_h264_task *)ctx->task->vaddr;
-   struct hva_buffer *tmp_frame;
u32 stuffing_bytes = 0;
int ret = 0;
 
@@ -1023,9 +1022,7 @@ static int hva_h264_encode(struct hva_ctx *pctx, struct 
hva_frame *frame,
   >bytesused);
 
/* switch reference & reconstructed frame */
-   tmp_frame = ctx->ref_frame;
-   ctx->ref_frame = ctx->rec_frame;
-   ctx->rec_frame = tmp_frame;
+   swap(ctx->ref_frame, ctx->rec_frame);
 
return 0;
 err:
-- 
2.7.4

Re: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1

[Gustavo A. R. Silva]

Hi Mauro,

On 04/26/2018 06:42 PM, Mauro Carvalho Chehab wrote:



I noticed you changed the status of this series from rejected to new.


Yes.


Also, there are other similar issues in media/pci/


Well, the issues will be there everywhere on all media drivers.

I marked your patches because I need to study it carefully, after
Peter's explanations. My plan is to do it next week. Still not
sure if the approach you took is the best one or not.

As I said, one possibility is to change the way v4l2-core handles
VIDIOC_ENUM_foo ioctls, but that would be make harder to -stable
backports.

I need a weekend to sleep on it.



I'm curious about how you finally resolved to handle these issues.

I noticed Smatch is no longer reporting them.

Thanks
--
Gustavo

Re: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1

[Gustavo A. R. Silva]

On 05/16/2018 08:14 PM, Gustavo A. R. Silva wrote:



On 05/15/2018 02:39 PM, Dan Carpenter wrote:

Dan,

These are all the Spectre media issues I see smatch is reporting in
linux-next-20180515:

drivers/media/cec/cec-pin-error-inj.c:170 cec_pin_error_inj_parse_line()
warn: potential spectre issue 'pin->error_inj_args'
drivers/media/dvb-core/dvb_ca_en50221.c:1479 
dvb_ca_en50221_io_write() warn:

potential spectre issue 'ca->slot_info' (local cap)
drivers/media/dvb-core/dvb_net.c:252 handle_one_ule_extension() warn:
potential spectre issue 'p->ule_next_hdr'

I pulled the latest changes from the smatch repository and compiled it.

I'm running smatch v0.5.0-4459-g2f66d40 now. Is this the latest version?

I wonder if there is anything I might be missing.



You'd need to rebuild the db (possibly twice but definitely once).



Hi Dan,

After rebuilding the db (once), these are all the Spectre media warnings 
I get:


drivers/media/pci/ddbridge/ddbridge-core.c:233 ddb_redirect() warn: 
potential spectre issue 'ddbs'
drivers/media/pci/ddbridge/ddbridge-core.c:243 ddb_redirect() warn: 
potential spectre issue 'pdev->port'
drivers/media/pci/ddbridge/ddbridge-core.c:252 ddb_redirect() warn: 
potential spectre issue 'idev->input'
drivers/media/dvb-core/dvb_ca_en50221.c:1400 
dvb_ca_en50221_io_do_ioctl() warn: potential spectre issue 
'ca->slot_info' (local cap)
drivers/media/dvb-core/dvb_ca_en50221.c:1479 dvb_ca_en50221_io_write() 
warn: potential spectre issue 'ca->slot_info' (local cap)
drivers/media/dvb-core/dvb_net.c:252 handle_one_ule_extension() warn: 
potential spectre issue 'p->ule_next_hdr'
drivers/media/dvb-core/dvb_net.c:1483 dvb_net_do_ioctl() warn: potential 
spectre issue 'dvbnet->device' (local cap)
drivers/media/cec/cec-pin-error-inj.c:170 cec_pin_error_inj_parse_line() 
warn: potential spectre issue 'pin->error_inj_args'


I just want to double check if you are getting the same output. In case 
you are getting the same, then what Mauro commented about these issues:


https://patchwork.linuxtv.org/project/linux-media/list/?submitter=7277

being resolved by commit 3ad3b7a2ebaefae37a7eafed0779324987ca5e56 seems 
to be correct.




Interesting, I've rebuild the db twice and now I get a total of 75 
Spectre warnings in drivers/media


--
Gustavo

Re: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1

[Gustavo A. R. Silva]

On 05/15/2018 09:16 AM, Dan Carpenter wrote:


I'm curious about how you finally resolved to handle these issues.

I noticed Smatch is no longer reporting them.


There was no direct fix for it, but maybe this patch has something
to do with the smatch error report cleanup:

commit 3ad3b7a2ebaefae37a7eafed0779324987ca5e56
Author: Sami Tolvanen 
Date:   Tue May 8 13:56:12 2018 -0400

 media: v4l2-ioctl: replace IOCTL_INFO_STD with stub functions
 
 This change removes IOCTL_INFO_STD and adds stub functions where

 needed using the DEFINE_V4L_STUB_FUNC macro. This fixes indirect call
 mismatches with Control-Flow Integrity, caused by calling standard
 ioctls using a function pointer that doesn't match the function type.
 
 Signed-off-by: Sami Tolvanen 

 Signed-off-by: Hans Verkuil 
 Signed-off-by: Mauro Carvalho Chehab 



Thanks, Mauro.



Possibly...  There was an ancient bug in Smatch's function pointer
handling.  I just pushed a fix for it now so the warning is there on
linux-next.



Dan,

These are all the Spectre media issues I see smatch is reporting in 
linux-next-20180515:


drivers/media/cec/cec-pin-error-inj.c:170 cec_pin_error_inj_parse_line() 
warn: potential spectre issue 'pin->error_inj_args'
drivers/media/dvb-core/dvb_ca_en50221.c:1479 dvb_ca_en50221_io_write() 
warn: potential spectre issue 'ca->slot_info' (local cap)
drivers/media/dvb-core/dvb_net.c:252 handle_one_ule_extension() warn: 
potential spectre issue 'p->ule_next_hdr'


I pulled the latest changes from the smatch repository and compiled it.

I'm running smatch v0.5.0-4459-g2f66d40 now. Is this the latest version?

I wonder if there is anything I might be missing.

Thanks
--
Gustavo

Re: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1

[Gustavo A. R. Silva]

On 05/17/2018 07:13 AM, Mauro Carvalho Chehab wrote:

Em Thu, 17 May 2018 08:43:24 -0300
Mauro Carvalho Chehab  escreveu:


On 05/15/2018 02:39 PM, Dan Carpenter wrote:
   

You'd need to rebuild the db (possibly twice but definitely once).


How? Here, I just pull from your git tree and do a "make". At most,
make clean; make.


Never mind. Found it using grep. I'm running this:

make allyesconfig
/devel/smatch/smatch_scripts/build_kernel_data.sh
/devel/smatch/smatch_scripts/build_kernel_data.sh


It seems that something is broken... getting this error/warning:

DBD::SQLite::db do failed: unrecognized token: "'end + strlen("
" at /devel/smatch/smatch_scripts/../smatch_data/db/fill_db_sql.pl line 32, 
 line 2938054.



Yep. I get the same warning multiple times.

BTW, Mauro, you sent a patch to fix an spectre v1 issue in this file 
yesterday: dvb_ca_en50221.c:1480, but it seems there is another instance 
of the same issue some lines above:


diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c

index 1310526..7edd9db 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1398,6 +1398,7 @@ static int dvb_ca_en50221_io_do_ioctl(struct file 
*file,


info->type = CA_CI_LINK;
info->flags = 0;
+   slot = array_index_nospec(slot, ca->slot_count + 1);
sl = >slot_info[slot];
if ((sl->slot_state != DVB_CA_SLOTSTATE_NONE) &&
(sl->slot_state != DVB_CA_SLOTSTATE_INVALID)) {


Thanks
--
Gustavo

Re: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1

[Gustavo A. R. Silva]

On 05/17/2018 01:08 PM, Gustavo A. R. Silva wrote:


BTW, Mauro, you sent a patch to fix an spectre v1 issue in this file 
yesterday: dvb_ca_en50221.c:1480, but it seems there is another instance 
of the same issue some lines above:


diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c

index 1310526..7edd9db 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1398,6 +1398,7 @@ static int dvb_ca_en50221_io_do_ioctl(struct file 
*file,


     info->type = CA_CI_LINK;
     info->flags = 0;
+   slot = array_index_nospec(slot, ca->slot_count + 1);
     sl = >slot_info[slot];
     if ((sl->slot_state != DVB_CA_SLOTSTATE_NONE) &&
     (sl->slot_state != DVB_CA_SLOTSTATE_INVALID)) {




Hi Mauro,

Just to let you know, I was running smatch during the weekend and the 
tool is still reporting all these Spectre media warnings (and a lot more):


https://patchwork.linuxtv.org/project/linux-media/list/?submitter=7277

Thanks
--
Gustavo

[media] duplicate code in media drivers

[Gustavo A. R. Silva]

Hi Mauro,

I found some duplicate code with the help of Coccinelle and Coverity. Notice 
that these are not code patches, they only point out the duplicate code in some 
media drivers:

diff -u -p drivers/media/pci/bt8xx/dvb-bt8xx.c 
/tmp/nothing/media/pci/bt8xx/dvb-bt8xx.c
--- drivers/media/pci/bt8xx/dvb-bt8xx.c
+++ /tmp/nothing/media/pci/bt8xx/dvb-bt8xx.c
@@ -389,9 +389,7 @@ static int advbt771_samsung_tdtc9251dh0_
else if (c->frequency < 6)
bs = 0x08;
else if (c->frequency < 73000)
-   bs = 0x08;
else
-   bs = 0x08;

pllbuf[0] = 0x61;
pllbuf[1] = div >> 8;
diff -u -p drivers/media/usb/dvb-usb/dib0700_devices.c 
/tmp/nothing/media/usb/dvb-usb/dib0700_devices.c
--- drivers/media/usb/dvb-usb/dib0700_devices.c
+++ /tmp/nothing/media/usb/dvb-usb/dib0700_devices.c
@@ -1741,13 +1741,6 @@ static int dib809x_tuner_attach(struct d
struct dib0700_adapter_state *st = adap->priv;
struct i2c_adapter *tun_i2c = 
st->dib8000_ops.get_i2c_master(adap->fe_adap[0].fe, 
DIBX000_I2C_INTERFACE_TUNER, 1);

-   if (adap->id == 0) {
-   if (dvb_attach(dib0090_register, adap->fe_adap[0].fe, tun_i2c, 
_dib0090_config) == NULL)
-   return -ENODEV;
-   } else {
-   if (dvb_attach(dib0090_register, adap->fe_adap[0].fe, tun_i2c, 
_dib0090_config) == NULL)
-   return -ENODEV;
-   }

st->set_param_save = adap->fe_adap[0].fe->ops.tuner_ops.set_params;
adap->fe_adap[0].fe->ops.tuner_ops.set_params = 
dib8096_set_param_override;
diff -u -p drivers/media/dvb-frontends/mb86a16.c 
/tmp/nothing/media/dvb-frontends/mb86a16.c
--- drivers/media/dvb-frontends/mb86a16.c
+++ /tmp/nothing/media/dvb-frontends/mb86a16.c
@@ -1466,9 +1466,7 @@ static int mb86a16_set_fe(struct mb86a16
wait_t = (1572864 + 
state->srate / 2) / state->srate;
if (state->srate < 5000)
/* FIXME ! , should be 
a long wait ! */
-   
msleep_interruptible(wait_t);
else
-   
msleep_interruptible(wait_t);

if (sync_chk(state, ) == 
0) {
iq_vt_set(state, 1);
diff -u -p drivers/media/dvb-frontends/au8522_decoder.c 
/tmp/nothing/media/dvb-frontends/au8522_decoder.c
--- drivers/media/dvb-frontends/au8522_decoder.c
+++ /tmp/nothing/media/dvb-frontends/au8522_decoder.c
@@ -280,14 +280,9 @@ static void setup_decoder_defaults(struc
AU8522_TOREGAAGC_REG0E5H_CVBS);
au8522_writereg(state, AU8522_REG016H, AU8522_REG016H_CVBS);

-   if (is_svideo) {
/* Despite what the table says, for the HVR-950q we still need
   to be in CVBS mode for the S-Video input (reason unknown). */
/* filter_coef_type = 3; */
-   filter_coef_type = 5;
-   } else {
-   filter_coef_type = 5;
-   }

/* Load the Video Decoder Filter Coefficients */
for (i = 0; i < NUM_FILTER_COEF; i++) {


I wonder if some of the cases above were intentionally coded that way or some 
code needs to be removed.

Thanks
--
Gustavo

[PATCH] media: si470x: fix potential Spectre variant 1

[Gustavo A. R. Silva]

band->index can be controlled by user-space, hence leading to
a potential exploitation of the Spectre variant 1 vulnerability.

This issue was detected with the help of Smatch:
drivers/media/radio/si470x/radio-si470x-common.c:758 
si470x_vidioc_enum_freq_bands() warn: potential spectre issue 'bands'

Fix this by sanitizing band->index before using it to index
`bands'

Notice that given that speculation windows are large, the policy is
to kill the speculation on the first load and not worry if it can be
completed with a dependent load/store [1].

[1] https://marc.info/?l=linux-kernel=152449131114778=2

Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/radio/si470x/radio-si470x-common.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/radio/si470x/radio-si470x-common.c 
b/drivers/media/radio/si470x/radio-si470x-common.c
index c40e175..e81f9aa 100644
--- a/drivers/media/radio/si470x/radio-si470x-common.c
+++ b/drivers/media/radio/si470x/radio-si470x-common.c
@@ -110,6 +110,10 @@
 /* kernel includes */
 #include "radio-si470x.h"
 
+/* Hardening for Spectre-v1 */
+#include 
+
+
 /**
  * Module Parameters
  **/
@@ -755,7 +759,7 @@ static int si470x_vidioc_enum_freq_bands(struct file *file, 
void *priv,
return -EINVAL;
if (band->index >= ARRAY_SIZE(bands))
return -EINVAL;
-   *band = bands[band->index];
+   *band = bands[array_index_nospec(band->index, ARRAY_SIZE(bands))];
return 0;
 }
 
-- 
2.7.4

[PATCH] media: dvb-bt8xx: remove duplicate code

[Gustavo A. R. Silva]

The same code is executed regardless of whether c->frequency < 6
or c->frequency < 73000 is true.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/pci/bt8xx/dvb-bt8xx.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/pci/bt8xx/dvb-bt8xx.c 
b/drivers/media/pci/bt8xx/dvb-bt8xx.c
index 5ef6e20..6b2a9e6 100644
--- a/drivers/media/pci/bt8xx/dvb-bt8xx.c
+++ b/drivers/media/pci/bt8xx/dvb-bt8xx.c
@@ -386,10 +386,6 @@ static int 
advbt771_samsung_tdtc9251dh0_tuner_calc_regs(struct dvb_frontend *fe,
bs = 0x02;
else if (c->frequency < 47000)
bs = 0x02;
-   else if (c->frequency < 6)
-   bs = 0x08;
-   else if (c->frequency < 73000)
-   bs = 0x08;
else
bs = 0x08;
 
-- 
2.7.4

[PATCH] media: dib0700: add code comment

[Gustavo A. R. Silva]

Add FIXME code comment:
/* FIXME: check if it is fe_adap[1] */

It is likely that it should be adap->fe_adap[1].fe in the second clause,
but this has never been verified.

Suggested-by: Mauro Carvalho Chehab <mche...@kernel.org>
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/usb/dvb-usb/dib0700_devices.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/usb/dvb-usb/dib0700_devices.c 
b/drivers/media/usb/dvb-usb/dib0700_devices.c
index c53a969..091389f 100644
--- a/drivers/media/usb/dvb-usb/dib0700_devices.c
+++ b/drivers/media/usb/dvb-usb/dib0700_devices.c
@@ -1745,6 +1745,7 @@ static int dib809x_tuner_attach(struct dvb_usb_adapter 
*adap)
if (dvb_attach(dib0090_register, adap->fe_adap[0].fe, tun_i2c, 
_dib0090_config) == NULL)
return -ENODEV;
} else {
+   /* FIXME: check if it is fe_adap[1] */
if (dvb_attach(dib0090_register, adap->fe_adap[0].fe, tun_i2c, 
_dib0090_config) == NULL)
return -ENODEV;
}
-- 
2.7.4

Re: [media] duplicate code in media drivers

[Gustavo A. R. Silva]

Mauro,

I already sent some patches based on your comments.

Thanks!
--
Gustavo

On 05/21/2018 03:14 PM, Mauro Carvalho Chehab wrote:

Em Mon, 21 May 2018 14:39:51 -0500
"Gustavo A. R. Silva" <gust...@embeddedor.com> escreveu:


Hi Mauro,

I found some duplicate code with the help of Coccinelle and Coverity. Notice 
that these are not code patches, they only point out the duplicate code in some 
media drivers:

diff -u -p drivers/media/pci/bt8xx/dvb-bt8xx.c 
/tmp/nothing/media/pci/bt8xx/dvb-bt8xx.c
--- drivers/media/pci/bt8xx/dvb-bt8xx.c
+++ /tmp/nothing/media/pci/bt8xx/dvb-bt8xx.c
@@ -389,9 +389,7 @@ static int advbt771_samsung_tdtc9251dh0_
 else if (c->frequency < 6)
 bs = 0x08;
 else if (c->frequency < 73000)
-   bs = 0x08;
 else
-   bs = 0x08;

 pllbuf[0] = 0x61;
 pllbuf[1] = div >> 8;



Hmm... I *suspect* that "bs" here controls the frequency range for the
tuner. Analog tuners have separate frequency regions that are controlled
via a register, into a 4 or 5 bytes I2C sequence. They're all somewhat
a clone of an old Philips design.

It should be safe to convert the "BS" sequence on something like:

if (c->frequency < 17300)
 bs = 0x01;
 else if (c->frequency < 47000)
 bs = 0x02;
 else
 bs = 0x08;




diff -u -p drivers/media/usb/dvb-usb/dib0700_devices.c 
/tmp/nothing/media/usb/dvb-usb/dib0700_devices.c
--- drivers/media/usb/dvb-usb/dib0700_devices.c
+++ /tmp/nothing/media/usb/dvb-usb/dib0700_devices.c
@@ -1741,13 +1741,6 @@ static int dib809x_tuner_attach(struct d
 struct dib0700_adapter_state *st = adap->priv;
 struct i2c_adapter *tun_i2c = 
st->dib8000_ops.get_i2c_master(adap->fe_adap[0].fe, 
DIBX000_I2C_INTERFACE_TUNER, 1);

-   if (adap->id == 0) {
-   if (dvb_attach(dib0090_register, adap->fe_adap[0].fe, tun_i2c, 
_dib0090_config) == NULL)
-   return -ENODEV;
-   } else {
-   if (dvb_attach(dib0090_register, adap->fe_adap[0].fe, tun_i2c, 
_dib0090_config) == NULL)
-   return -ENODEV;
-   }


I'm almost sure that, on the second if, it should be adap->fe_adap[1].fe.
I tried in the past to check this, but didn't got an answer from the one
that wrote the code.

Maybe we could add a /* FIXME: check if it is fe_adap[1] */ on the
second clause.



 st->set_param_save = adap->fe_adap[0].fe->ops.tuner_ops.set_params;
 adap->fe_adap[0].fe->ops.tuner_ops.set_params = 
dib8096_set_param_override;
diff -u -p drivers/media/dvb-frontends/mb86a16.c 
/tmp/nothing/media/dvb-frontends/mb86a16.c
--- drivers/media/dvb-frontends/mb86a16.c
+++ /tmp/nothing/media/dvb-frontends/mb86a16.c
@@ -1466,9 +1466,7 @@ static int mb86a16_set_fe(struct mb86a16
 wait_t = (1572864 + 
state->srate / 2) / state->srate;
 if (state->srate < 5000)
 /* FIXME ! , should be 
a long wait ! */
-   
msleep_interruptible(wait_t);
 else
-   
msleep_interruptible(wait_t);


I suspect that the goal here is to point that sleeping for
(1572864 + state->srate / 2) / state->srate when srate is low will mean
that it will take a lot of time to converge (probably causing timeout at
userspace).

Basically, if srate is < 5000, the sleep time will be between
314 and 1575364 ms. The worse case scenario - although not realistic,
in practice - is to wait up to 26 seconds. This is a very long time!

Probably, the right fix here would be to check if wait_t is bigger than
a certain amount of time. If so, return an error.

I'm not against removing the if, but, if so, better to add a /* FIXME */
block explaining that.

That's said, this is an old device. I doubt anyone would fix it.




 if (sync_chk(state, ) == 
0) {
 iq_vt_set(state, 1);
diff -u -p drivers/media/dvb-frontends/au8522_decoder.c 
/tmp/nothing/media/dvb-frontends/au8522_decoder.c
--- drivers/media/dvb-frontends/au8522_decoder.c
+++ /tmp/nothing/media/dvb-frontends/au8522_decoder.c
@@ -280,14 +280,9 @@ static void setup_decoder_defaults(struc
 AU8522_TOREGAAGC_REG0E5H_CVBS);
 au8522_writereg(state, AU8522_REG016H, AU8522_REG016H_CVBS);

-   if (is_svideo) {
 /* Despite what the table says, for the HVR-950q we still need
to be in CVBS mode for the S-Video input (reason unknown). 
*/
 /* filter_coef_type = 3; *

[PATCH] au8522: remove duplicate code

[Gustavo A. R. Silva]

This code has been there for nine years now, and it has been
working "good enough" since then [1].

Remove duplicate code by getting rid of the if-else statement.

[1] https://marc.info/?l=linux-kernel=152693550225081=2

Cc: Devin Heitmueller <dheitmuel...@kernellabs.com>
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/dvb-frontends/au8522_decoder.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/media/dvb-frontends/au8522_decoder.c 
b/drivers/media/dvb-frontends/au8522_decoder.c
index 343dc92..f285096 100644
--- a/drivers/media/dvb-frontends/au8522_decoder.c
+++ b/drivers/media/dvb-frontends/au8522_decoder.c
@@ -280,14 +280,12 @@ static void setup_decoder_defaults(struct au8522_state 
*state, bool is_svideo)
AU8522_TOREGAAGC_REG0E5H_CVBS);
au8522_writereg(state, AU8522_REG016H, AU8522_REG016H_CVBS);
 
-   if (is_svideo) {
-   /* Despite what the table says, for the HVR-950q we still need
-  to be in CVBS mode for the S-Video input (reason unknown). */
-   /* filter_coef_type = 3; */
-   filter_coef_type = 5;
-   } else {
-   filter_coef_type = 5;
-   }
+   /*
+* Despite what the table says, for the HVR-950q we still need
+* to be in CVBS mode for the S-Video input (reason unknown).
+*/
+   /* filter_coef_type = 3; */
+   filter_coef_type = 5;
 
/* Load the Video Decoder Filter Coefficients */
for (i = 0; i < NUM_FILTER_COEF; i++) {
-- 
2.7.4

Re: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1

[Gustavo A. R. Silva]

On 05/15/2018 02:39 PM, Dan Carpenter wrote:

Dan,

These are all the Spectre media issues I see smatch is reporting in
linux-next-20180515:

drivers/media/cec/cec-pin-error-inj.c:170 cec_pin_error_inj_parse_line()
warn: potential spectre issue 'pin->error_inj_args'
drivers/media/dvb-core/dvb_ca_en50221.c:1479 dvb_ca_en50221_io_write() warn:
potential spectre issue 'ca->slot_info' (local cap)
drivers/media/dvb-core/dvb_net.c:252 handle_one_ule_extension() warn:
potential spectre issue 'p->ule_next_hdr'

I pulled the latest changes from the smatch repository and compiled it.

I'm running smatch v0.5.0-4459-g2f66d40 now. Is this the latest version?

I wonder if there is anything I might be missing.



You'd need to rebuild the db (possibly twice but definitely once).



Hi Dan,

After rebuilding the db (once), these are all the Spectre media warnings 
I get:


drivers/media/pci/ddbridge/ddbridge-core.c:233 ddb_redirect() warn: 
potential spectre issue 'ddbs'
drivers/media/pci/ddbridge/ddbridge-core.c:243 ddb_redirect() warn: 
potential spectre issue 'pdev->port'
drivers/media/pci/ddbridge/ddbridge-core.c:252 ddb_redirect() warn: 
potential spectre issue 'idev->input'
drivers/media/dvb-core/dvb_ca_en50221.c:1400 
dvb_ca_en50221_io_do_ioctl() warn: potential spectre issue 
'ca->slot_info' (local cap)
drivers/media/dvb-core/dvb_ca_en50221.c:1479 dvb_ca_en50221_io_write() 
warn: potential spectre issue 'ca->slot_info' (local cap)
drivers/media/dvb-core/dvb_net.c:252 handle_one_ule_extension() warn: 
potential spectre issue 'p->ule_next_hdr'
drivers/media/dvb-core/dvb_net.c:1483 dvb_net_do_ioctl() warn: potential 
spectre issue 'dvbnet->device' (local cap)
drivers/media/cec/cec-pin-error-inj.c:170 cec_pin_error_inj_parse_line() 
warn: potential spectre issue 'pin->error_inj_args'


I just want to double check if you are getting the same output. In case 
you are getting the same, then what Mauro commented about these issues:


https://patchwork.linuxtv.org/project/linux-media/list/?submitter=7277

being resolved by commit 3ad3b7a2ebaefae37a7eafed0779324987ca5e56 seems 
to be correct.


Thanks
--
Gustavo

Re: [media] duplicate code in media drivers

[Gustavo A. R. Silva]

On 05/21/2018 03:44 PM, Devin Heitmueller wrote:

diff -u -p drivers/media/dvb-frontends/au8522_decoder.c 
/tmp/nothing/media/dvb-frontends/au8522_decoder.c
--- drivers/media/dvb-frontends/au8522_decoder.c
+++ /tmp/nothing/media/dvb-frontends/au8522_decoder.c
@@ -280,14 +280,9 @@ static void setup_decoder_defaults(struc
 AU8522_TOREGAAGC_REG0E5H_CVBS);
 au8522_writereg(state, AU8522_REG016H, AU8522_REG016H_CVBS);

-   if (is_svideo) {
 /* Despite what the table says, for the HVR-950q we still need
to be in CVBS mode for the S-Video input (reason unknown). 
*/
 /* filter_coef_type = 3; */
-   filter_coef_type = 5;
-   } else {
-   filter_coef_type = 5;
-   }


Better ask Devin about this (c/c).


This was a case where the implementation didn't match the datasheet,
and it wasn't clear why the filter coefficients weren't working
properly.  Essentially I should have labeled that as a TODO or FIXME
when I disabled the "right" value and forced it to always be five.  It
was also likely that the filter coefficients would need to differ if
taking video over the IF interface as opposed to CVBS/S-video, which
is why I didn't want to get rid of the logic entirely.  That said, the
only product I've ever seen with the tda18271 mated to the au8522 will
likely never be supported for analog video under Linux for unrelated
reasons.

That said, it's worked "good enough" since I wrote the code nine years
ago, so if somebody wants to submit a patch to either get rid of the
if() statement or mark it as a FIXME that will likely never actually
get fixed, I wouldn't have an objection to either.



Devin,

I've sent a patch based on your feedback.

Thanks!
--
Gustavo

Re: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1

[Gustavo A. R. Silva]

Hi Mauro,

On 04/23/2018 02:17 PM, Mauro Carvalho Chehab wrote:

Em Mon, 23 Apr 2018 14:11:02 -0500

Thanks, I 'll mark this series as rejected at patchwork.linuxtv.org.
Please feel free to resubmit any patch if they represent a real
threat, adding a corresponding description about the threat scenario
at the body of the e-mail.


Sorry for the noise and thanks for the feedback.


Anytime.



I noticed you changed the status of this series from rejected to new.

Also, there are other similar issues in media/pci/

I can write proper patches for all of them if you agree those are not 
False Positives:


diff --git a/drivers/media/pci/cx18/cx18-ioctl.c 
b/drivers/media/pci/cx18/cx18-ioctl.c

index 80b902b..63f4388 100644
--- a/drivers/media/pci/cx18/cx18-ioctl.c
+++ b/drivers/media/pci/cx18/cx18-ioctl.c
@@ -36,6 +36,8 @@
 #include 
 #include 

+#include 
+
 u16 cx18_service2vbi(int type)
 {
switch (type) {
@@ -488,8 +490,9 @@ static int cx18_enum_fmt_vid_cap(struct file *file, 
void *fh,

},
};

-   if (fmt->index > ARRAY_SIZE(formats) - 1)
+   if (fmt->index >= ARRAY_SIZE(formats))
return -EINVAL;
+   fmt->index = array_index_nospec(fmt->index, ARRAY_SIZE(formats));
*fmt = formats[fmt->index];
return 0;
 }
diff --git a/drivers/media/pci/saa7134/saa7134-video.c 
b/drivers/media/pci/saa7134/saa7134-video.c

index 1a50ec9..d93cf09 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -30,6 +30,8 @@
 #include 
 #include 

+#include 
+
 /* -- */

 unsigned int video_debug;
@@ -1819,6 +1821,8 @@ static int saa7134_enum_fmt_vid_cap(struct file 
*file, void  *priv,

if (f->index >= FORMATS)
return -EINVAL;

+   f->index = array_index_nospec(f->index, FORMATS);
+
strlcpy(f->description, formats[f->index].name,
sizeof(f->description));

diff --git a/drivers/media/pci/tw68/tw68-video.c 
b/drivers/media/pci/tw68/tw68-video.c

index 8c1f4a0..a6cfb4b 100644
--- a/drivers/media/pci/tw68/tw68-video.c
 #include 
 #include 

+#include 
+
 #include "tw68.h"
 #include "tw68-reg.h"

@@ -789,6 +791,8 @@ static int tw68_enum_fmt_vid_cap(struct file *file, 
void  *priv,

if (f->index >= FORMATS)
return -EINVAL;

+   f->index = array_index_nospec(f->index, FORMATS);
+
strlcpy(f->description, formats[f->index].name,
sizeof(f->description));

diff --git a/drivers/media/pci/tw686x/tw686x-video.c 
b/drivers/media/pci/tw686x/tw686x-video.c

index c3fafa9..281d722 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -25,6 +25,8 @@
 #include "tw686x.h"
 #include "tw686x-regs.h"

+#include 
+
 #define TW686X_INPUTS_PER_CH   4
 #define TW686X_VIDEO_WIDTH 720
 #define TW686X_VIDEO_HEIGHT(id)((id & V4L2_STD_525_60) 
? 480 : 576)
@@ -981,6 +983,7 @@ static int tw686x_enum_fmt_vid_cap(struct file 
*file, void *priv,

 {
if (f->index >= ARRAY_SIZE(formats))
return -EINVAL;
+   f->index = array_index_nospec(f->index, ARRAY_SIZE(formats));
f->pixelformat = formats[f->index].fourcc;
return 0;
 }


Thanks
--
Gustavo

Re: [PATCH][next] media: ispstat: don't dereference user_cfg before a null check

[Gustavo A. R. Silva]

Hi Sakari,

On 04/30/2018 10:15 AM, Sakari Ailus wrote:

Isn't there a guarantee that new_buf won't be NULL ? The new_buf pointer comes
from the parg variable in video_usercopy(), which should always point to a
valid buffer given that the ioctl number specifies a non-zero size.


Fair question. After looking at the code, I agree with you; there should be
no reason to perform the check in the first place. It may have been that
the function has been used differently in the past but the check should be
rather removed now.

I'll drop the patch.



Please, if the check isn't needed anymore, make sure it is removed.

This helps to reduce the number of false positives reported by static 
analyzers.


Thanks
--
Gustavo

Re: [PATCH] staging: imx-media-vdic: fix inconsistent IS_ERR and PTR_ERR

[Gustavo A. R. Silva]

Quoting Arnd Bergmann <a...@arndb.de>:


On Wed, Jan 24, 2018 at 1:43 AM, Gustavo A. R. Silva
<gust...@embeddedor.com> wrote:

Fix inconsistent IS_ERR and PTR_ERR in vdic_get_ipu_resources.
The proper pointer to be passed as argument is ch.

This issue was detected with the help of Coccinelle.

Fixes: 0b2e9e7947e7 ("media: staging/imx: remove confusing  
IS_ERR_OR_NULL usage")

Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>


good catch!



:)


Acked-by: Arnd Bergmann <a...@arndb.de>


Thanks, Arnd.
--
Gustavo

[PATCH 7/8] platform: sh_veu: fix potential integer overflow in sh_veu_colour_offset

[Gustavo A. R. Silva]

Cast left and top to dma_addr_t in order to avoid a potential integer
overflow. This variable is being used in a context that expects an
expression of type dma_addr_t (u64).

Addresses-Coverity-ID: 1056807 ("Unintentional integer overflow")
Addresses-Coverity-ID: 1056808 ("Unintentional integer overflow")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/platform/sh_veu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/sh_veu.c b/drivers/media/platform/sh_veu.c
index 976ea0b..e2795d0 100644
--- a/drivers/media/platform/sh_veu.c
+++ b/drivers/media/platform/sh_veu.c
@@ -520,8 +520,8 @@ static void sh_veu_colour_offset(struct sh_veu_dev *veu, 
struct sh_veu_vfmt *vfm
/* dst_left and dst_top validity will be verified in CROP / COMPOSE */
unsigned int left = vfmt->frame.left & ~0x03;
unsigned int top = vfmt->frame.top;
-   dma_addr_t offset = ((left * veu->vfmt_out.fmt->depth) >> 3) +
-   top * veu->vfmt_out.bytesperline;
+   dma_addr_t offset = (((dma_addr_t)left * veu->vfmt_out.fmt->depth) >> 
3) +
+   (dma_addr_t)top * veu->vfmt_out.bytesperline;
unsigned int y_line;
 
vfmt->offset_y = offset;
-- 
2.7.4

[PATCH 5/8] pci: cx88-input: fix potential integer overflow

[Gustavo A. R. Silva]

Cast ir->polling to ktime_t in order to avoid a potential integer
overflow. This variable is being used in a context that expects
an expression of type ktime_t (s64).

Addresses-Coverity-ID: 1392628 ("Unintentional integer overflow")
Addresses-Coverity-ID: 1392630 ("Unintentional integer overflow")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/pci/cx88/cx88-input.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/cx88/cx88-input.c 
b/drivers/media/pci/cx88/cx88-input.c
index 4e9953e..096b350 100644
--- a/drivers/media/pci/cx88/cx88-input.c
+++ b/drivers/media/pci/cx88/cx88-input.c
@@ -180,7 +180,7 @@ static enum hrtimer_restart cx88_ir_work(struct hrtimer 
*timer)
struct cx88_IR *ir = container_of(timer, struct cx88_IR, timer);
 
cx88_ir_handle_key(ir);
-   missed = hrtimer_forward_now(>timer, ir->polling * 100);
+   missed = hrtimer_forward_now(>timer, (ktime_t)ir->polling * 
100);
if (missed > 1)
ir_dprintk("Missed ticks %ld\n", missed - 1);
 
@@ -200,7 +200,7 @@ static int __cx88_ir_start(void *priv)
if (ir->polling) {
hrtimer_init(>timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
ir->timer.function = cx88_ir_work;
-   hrtimer_start(>timer, ir->polling * 100,
+   hrtimer_start(>timer, (ktime_t)ir->polling * 100,
  HRTIMER_MODE_REL);
}
if (ir->sampling) {
-- 
2.7.4

Re: [PATCH 8/8] platform: vivid-cec: fix potential integer overflow in vivid_cec_pin_adap_events

[Gustavo A. R. Silva]

Hi Hans,

Quoting Hans Verkuil <hverk...@xs4all.nl>:


Hi Gustavo,

On 01/30/2018 01:33 AM, Gustavo A. R. Silva wrote:

Cast len to const u64 in order to avoid a potential integer
overflow. This variable is being used in a context that expects
an expression of type const u64.

Addresses-Coverity-ID: 1454996 ("Unintentional integer overflow")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/platform/vivid/vivid-cec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vivid/vivid-cec.c  
b/drivers/media/platform/vivid/vivid-cec.c

index b55d278..30240ab 100644
--- a/drivers/media/platform/vivid/vivid-cec.c
+++ b/drivers/media/platform/vivid/vivid-cec.c
@@ -83,7 +83,7 @@ static void vivid_cec_pin_adap_events(struct  
cec_adapter *adap, ktime_t ts,

if (adap == NULL)
return;
ts = ktime_sub_us(ts, (CEC_TIM_START_BIT_TOTAL +
-  len * 10 * CEC_TIM_DATA_BIT_TOTAL));
+  (const u64)len * 10 * CEC_TIM_DATA_BIT_TOTAL));


This makes no sense. Certainly the const part is pointless. And given that
len is always <= 16 there definitely is no overflow.



Yeah, I understand your point and I know there is no chance of an  
overflow in this particular case.



I don't really want this cast in the code.

Sorry,



I'm working through all the Linux kernel Coverity reports, and I  
thought of sending a patch for this because IMHO it doesn't hurt to  
give the compiler complete information about the arithmetic in which  
an expression is intended to be evaluated.


I agree that the _const_ part is a bit odd. What do you think about  
the cast to u64 alone?


I appreciate your feedback.

Thanks
--
Gustavo

[PATCH 1/8] rtl2832: fix potential integer overflow in rtl2832_set_frontend

[Gustavo A. R. Silva]

Cast dev->pdata->clk to u64 in order to avoid a potential integer
overflow. This variable is being used in a context that expects
an expression of type u64.

Addresses-Coverity-ID: 1271223 ("Unintentional integer overflow")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/dvb-frontends/rtl2832.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-frontends/rtl2832.c 
b/drivers/media/dvb-frontends/rtl2832.c
index 94bf5b7..eefc301 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -498,7 +498,7 @@ static int rtl2832_set_frontend(struct dvb_frontend *fe)
* RSAMP_RATIO = floor(CrystalFreqHz * 7 * pow(2, 22)
*   / ConstWithBandwidthMode)
*/
-   num = dev->pdata->clk * 7;
+   num = (u64)dev->pdata->clk * 7;
num *= 0x40;
num = div_u64(num, bw_mode);
resamp_ratio =  num & 0x3ff;
@@ -511,7 +511,7 @@ static int rtl2832_set_frontend(struct dvb_frontend *fe)
*   / (CrystalFreqHz * 7))
*/
num = bw_mode << 20;
-   num2 = dev->pdata->clk * 7;
+   num2 = (u64)dev->pdata->clk * 7;
num = div_u64(num, num2);
num = -num;
cfreq_off_ratio = num & 0xf;
-- 
2.7.4

[PATCH 8/8] platform: vivid-cec: fix potential integer overflow in vivid_cec_pin_adap_events

[Gustavo A. R. Silva]

Cast len to const u64 in order to avoid a potential integer
overflow. This variable is being used in a context that expects
an expression of type const u64.

Addresses-Coverity-ID: 1454996 ("Unintentional integer overflow")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/platform/vivid/vivid-cec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vivid/vivid-cec.c 
b/drivers/media/platform/vivid/vivid-cec.c
index b55d278..30240ab 100644
--- a/drivers/media/platform/vivid/vivid-cec.c
+++ b/drivers/media/platform/vivid/vivid-cec.c
@@ -83,7 +83,7 @@ static void vivid_cec_pin_adap_events(struct cec_adapter 
*adap, ktime_t ts,
if (adap == NULL)
return;
ts = ktime_sub_us(ts, (CEC_TIM_START_BIT_TOTAL +
-  len * 10 * CEC_TIM_DATA_BIT_TOTAL));
+  (const u64)len * 10 * CEC_TIM_DATA_BIT_TOTAL));
cec_queue_pin_cec_event(adap, false, ts);
ts = ktime_add_us(ts, CEC_TIM_START_BIT_LOW);
cec_queue_pin_cec_event(adap, true, ts);
-- 
2.7.4

[PATCH 4/8] i2c: ov9650: fix potential integer overflow in __ov965x_set_frame_interval

[Gustavo A. R. Silva]

Cast fi->interval.numerator to u64 in order to avoid a potential integer
overflow. This variable is being used in a context that expects an
expression of type u64.

Addresses-Coverity-ID: 1324146 ("Unintentional integer overflow")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/i2c/ov9650.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index e519f27..c674a49 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -1130,7 +1130,7 @@ static int __ov965x_set_frame_interval(struct ov965x 
*ov965x,
if (fi->interval.denominator == 0)
return -EINVAL;
 
-   req_int = (u64)(fi->interval.numerator * 1) /
+   req_int = (u64)fi->interval.numerator * 1 /
fi->interval.denominator;
 
for (i = 0; i < ARRAY_SIZE(ov965x_intervals); i++) {
-- 
2.7.4

[PATCH 3/8] i2c: max2175: fix potential integer overflow in max2175_set_nco_freq

[Gustavo A. R. Silva]

Cast expression (clock_rate - abs_nco_freq) to s64 in order to avoid
a potential integer overflow. This variable is being used in a
context that expects an expression of type s64.

Addresses-Coverity-ID: 1446589 ("Unintentional integer overflow")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/i2c/max2175.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/max2175.c b/drivers/media/i2c/max2175.c
index 2f1966b..3401ae6 100644
--- a/drivers/media/i2c/max2175.c
+++ b/drivers/media/i2c/max2175.c
@@ -643,7 +643,7 @@ static int max2175_set_nco_freq(struct max2175 *ctx, s32 
nco_freq)
if (abs_nco_freq < clock_rate / 2) {
nco_val_desired = 2 * nco_freq;
} else {
-   nco_val_desired = 2 * (clock_rate - abs_nco_freq);
+   nco_val_desired = 2 * (s64)(clock_rate - abs_nco_freq);
if (nco_freq < 0)
nco_val_desired = -nco_val_desired;
}
-- 
2.7.4

[PATCH 6/8] rockchip/rga: fix potential integer overflow in rga_buf_map

[Gustavo A. R. Silva]

Cast p to dma_addr_t in order to avoid a potential integer overflow.
This variable is being used in a context that expects an expression
of type dma_addr_t (u64).

Addresses-Coverity-ID: 1458347 ("Unintentional integer overflow")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
 drivers/media/platform/rockchip/rga/rga-buf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c 
b/drivers/media/platform/rockchip/rga/rga-buf.c
index 49cacc7..3dd29b2 100644
--- a/drivers/media/platform/rockchip/rga/rga-buf.c
+++ b/drivers/media/platform/rockchip/rga/rga-buf.c
@@ -140,7 +140,7 @@ void rga_buf_map(struct vb2_buffer *vb)
address = sg_phys(sgl);
 
for (p = 0; p < len; p++) {
-   dma_addr_t phys = address + (p << PAGE_SHIFT);
+   dma_addr_t phys = address + ((dma_addr_t)p << 
PAGE_SHIFT);
 
pages[mapped_size + p] = phys;
}
-- 
2.7.4

[PATCH 0/8] fix potential integer overflows

[Gustavo A. R. Silva]

This patchset aims to fix potential integer overflows reported
by Coverity.

In all cases the potential overflowing expressions are evaluated
using 32-bit arithmetic before being used in contexts that expect
a 64-bit arithmetic. So a cast to the proper type was added to each
of those expressions in order to avoid any potential integer overflow.

This also gives the compiler complete information about the proper
arithmetic for each expression and improves code quality.

Addresses the following Coverity IDs reported as
"Unintentional integer overflow" issues:

200604, 1056807, 1056808, 1271223,
1324146, 1392628, 1392630, 1446589,
1454996, 1458347.

Thank you

Gustavo A. R. Silva (8):
  rtl2832: fix potential integer overflow
  dvb-frontends: ves1820: fix potential integer overflow
  i2c: max2175: fix potential integer overflow in max2175_set_nco_freq
  i2c: ov9650: fix potential integer overflow in
__ov965x_set_frame_interval
  pci: cx88-input: fix potential integer overflow
  rockchip/rga: fix potential integer overflow in rga_buf_map
  platform: sh_veu: fix potential integer overflow in
sh_veu_colour_offset
  platform: vivid-cec: fix potential integer overflow in
vivid_cec_pin_adap_events

 drivers/media/dvb-frontends/rtl2832.c | 4 ++--
 drivers/media/dvb-frontends/ves1820.c | 2 +-
 drivers/media/i2c/max2175.c   | 2 +-
 drivers/media/i2c/ov9650.c| 2 +-
 drivers/media/pci/cx88/cx88-input.c   | 4 ++--
 drivers/media/platform/rockchip/rga/rga-buf.c | 2 +-
 drivers/media/platform/sh_veu.c   | 4 ++--
 drivers/media/platform/vivid/vivid-cec.c  | 2 +-
 8 files changed, 11 insertions(+), 11 deletions(-)

-- 
2.7.4