* [PATCH 1/4] tc358743: don't use variable length array for I2C writes
@ 2015-08-11 15:18 Mauro Carvalho Chehab
2015-08-11 15:18 ` [PATCH 2/4] ov9650: remove an extra space Mauro Carvalho Chehab
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-11 15:18 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Mats Randgaard
drivers/media/i2c/tc358743.c:148:19: warning: Variable length array is used.
As the maximum size is 1026, we can't use dynamic var, as it
would otherwise spend 1056 bytes of the stack at i2c_wr() function.
So, allocate a buffer with the allowed maximum size together with
the state var.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 2e926317d7e9..fe42c9a1cb78 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -59,6 +59,9 @@ MODULE_LICENSE("GPL");
#define EDID_NUM_BLOCKS_MAX 8
#define EDID_BLOCK_SIZE 128
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE (EDID_NUM_BLOCKS_MAX * EDID_BLOCK_SIZE + 2)
+
static const struct v4l2_dv_timings_cap tc358743_timings_cap = {
.type = V4L2_DV_BT_656_1120,
/* keep this initialization for compatibility with GCC < 4.4.6 */
@@ -94,6 +97,9 @@ struct tc358743_state {
/* edid */
u8 edid_blocks_written;
+ /* used by i2c_wr() */
+ u8 wr_data[MAX_XFER_SIZE];
+
struct v4l2_dv_timings timings;
u32 mbus_fmt_code;
@@ -143,9 +149,13 @@ static void i2c_wr(struct v4l2_subdev *sd, u16 reg, u8 *values, u32 n)
{
struct tc358743_state *state = to_state(sd);
struct i2c_client *client = state->i2c_client;
+ u8 *data = state->wr_data;
int err, i;
struct i2c_msg msg;
- u8 data[2 + n];
+
+ if ((2 + n) > sizeof(state->wr_data))
+ v4l2_warn(sd, "i2c wr reg=%04x: len=%d is too big!\n",
+ reg, 2 + n);
msg.addr = client->addr;
msg.buf = data;
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] ov9650: remove an extra space
2015-08-11 15:18 [PATCH 1/4] tc358743: don't use variable length array for I2C writes Mauro Carvalho Chehab
@ 2015-08-11 15:18 ` Mauro Carvalho Chehab
2015-08-11 15:18 ` [PATCH 3/4] ov2659: get rid of unused values Mauro Carvalho Chehab
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-11 15:18 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Hans Verkuil,
Prabhakar Lad, Sakari Ailus, Laurent Pinchart, Boris BREZILLON
drivers/media/i2c/ov9650.c:1439 ov965x_detect_sensor() warn: inconsistent indenting
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 2bc473385c91..e691bba1945b 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -1436,7 +1436,7 @@ static int ov965x_detect_sensor(struct v4l2_subdev *sd)
int ret;
mutex_lock(&ov965x->lock);
- __ov965x_set_power(ov965x, 1);
+ __ov965x_set_power(ov965x, 1);
usleep_range(25000, 26000);
/* Check sensor revision */
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] ov2659: get rid of unused values
2015-08-11 15:18 [PATCH 1/4] tc358743: don't use variable length array for I2C writes Mauro Carvalho Chehab
2015-08-11 15:18 ` [PATCH 2/4] ov9650: remove an extra space Mauro Carvalho Chehab
@ 2015-08-11 15:18 ` Mauro Carvalho Chehab
2015-08-11 15:18 ` [PATCH 4/4] sr030pc30: don't read a new pointer Mauro Carvalho Chehab
2015-08-12 12:24 ` [PATCH 1/4] tc358743: don't use variable length array for I2C writes Mats Randgaard (matrandg)
3 siblings, 0 replies; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-11 15:18 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Prabhakar Lad
Why to store the chosed values for prediv, postdiv and mult if
those won't be used?
drivers/media/i2c/ov2659.c: In function 'ov2659_pll_calc_params':
drivers/media/i2c/ov2659.c:912:35: warning: variable 's_mult' set but not used [-Wunused-but-set-variable]
u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
^
drivers/media/i2c/ov2659.c:912:20: warning: variable 's_postdiv' set but not used [-Wunused-but-set-variable]
u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
^
drivers/media/i2c/ov2659.c:912:6: warning: variable 's_prediv' set but not used [-Wunused-but-set-variable]
u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
^
This is likely some leftover from some past change.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
index 6edffc7b74e3..49109f4f5bb4 100644
--- a/drivers/media/i2c/ov2659.c
+++ b/drivers/media/i2c/ov2659.c
@@ -909,7 +909,6 @@ static void ov2659_pll_calc_params(struct ov2659 *ov2659)
u8 ctrl1_reg = 0, ctrl2_reg = 0, ctrl3_reg = 0;
struct i2c_client *client = ov2659->client;
unsigned int desired = pdata->link_frequency;
- u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
u32 prediv, postdiv, mult;
u32 bestdelta = -1;
u32 delta, actual;
@@ -929,9 +928,6 @@ static void ov2659_pll_calc_params(struct ov2659 *ov2659)
if ((delta < bestdelta) || (bestdelta == -1)) {
bestdelta = delta;
- s_mult = mult;
- s_prediv = prediv;
- s_postdiv = postdiv;
ctrl1_reg = ctrl1[i].reg;
ctrl2_reg = mult;
ctrl3_reg = ctrl3[j].reg;
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] sr030pc30: don't read a new pointer
2015-08-11 15:18 [PATCH 1/4] tc358743: don't use variable length array for I2C writes Mauro Carvalho Chehab
2015-08-11 15:18 ` [PATCH 2/4] ov9650: remove an extra space Mauro Carvalho Chehab
2015-08-11 15:18 ` [PATCH 3/4] ov2659: get rid of unused values Mauro Carvalho Chehab
@ 2015-08-11 15:18 ` Mauro Carvalho Chehab
2015-08-12 12:24 ` [PATCH 1/4] tc358743: don't use variable length array for I2C writes Mats Randgaard (matrandg)
3 siblings, 0 replies; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-11 15:18 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Hans Verkuil,
Prabhakar Lad, Guennadi Liakhovetski, Sakari Ailus,
Boris BREZILLON, Ricardo Ribalda Delgado
sr030pc30_get_fmt() can only succeed if both info->curr_win and
info->curr_fmt are not NULL.
If one of those vars are null, the curent code would call:
ret = sr030pc30_set_params(sd);
If the curr_win is null, it will return -EINVAL, as it would be
expected. However, if curr_fmt is NULL, the function won't
set it.
The code will then try to read from it:
mf->code = info->curr_fmt->code;
mf->colorspace = info->curr_fmt->colorspace;
with obviouly won't work.
This got reported by smatch:
drivers/media/i2c/sr030pc30.c:505 sr030pc30_get_fmt() error: we previously assumed 'info->curr_win' could be null (see line 499)
drivers/media/i2c/sr030pc30.c:507 sr030pc30_get_fmt() error: we previously assumed 'info->curr_fmt' could be null (see line 499)
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/i2c/sr030pc30.c b/drivers/media/i2c/sr030pc30.c
index 229dc76c44a5..47ebe27cb00e 100644
--- a/drivers/media/i2c/sr030pc30.c
+++ b/drivers/media/i2c/sr030pc30.c
@@ -496,11 +496,8 @@ static int sr030pc30_get_fmt(struct v4l2_subdev *sd,
mf = &format->format;
- if (!info->curr_win || !info->curr_fmt) {
- ret = sr030pc30_set_params(sd);
- if (ret)
- return ret;
- }
+ if (!info->curr_win || !info->curr_fmt)
+ return -EINVAL;
mf->width = info->curr_win->width;
mf->height = info->curr_win->height;
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/4] tc358743: don't use variable length array for I2C writes
2015-08-11 15:18 [PATCH 1/4] tc358743: don't use variable length array for I2C writes Mauro Carvalho Chehab
` (2 preceding siblings ...)
2015-08-11 15:18 ` [PATCH 4/4] sr030pc30: don't read a new pointer Mauro Carvalho Chehab
@ 2015-08-12 12:24 ` Mats Randgaard (matrandg)
3 siblings, 0 replies; 6+ messages in thread
From: Mats Randgaard (matrandg) @ 2015-08-12 12:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List; +Cc: Mauro Carvalho Chehab
On 08/11/2015 05:18 PM, Mauro Carvalho Chehab wrote:
> drivers/media/i2c/tc358743.c:148:19: warning: Variable length array is used.
>
> As the maximum size is 1026, we can't use dynamic var, as it
> would otherwise spend 1056 bytes of the stack at i2c_wr() function.
>
> So, allocate a buffer with the allowed maximum size together with
> the state var.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index 2e926317d7e9..fe42c9a1cb78 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -59,6 +59,9 @@ MODULE_LICENSE("GPL");
> #define EDID_NUM_BLOCKS_MAX 8
> #define EDID_BLOCK_SIZE 128
>
> +/* Max transfer size done by I2C transfer functions */
> +#define MAX_XFER_SIZE (EDID_NUM_BLOCKS_MAX * EDID_BLOCK_SIZE + 2)
> +
> static const struct v4l2_dv_timings_cap tc358743_timings_cap = {
> .type = V4L2_DV_BT_656_1120,
> /* keep this initialization for compatibility with GCC < 4.4.6 */
> @@ -94,6 +97,9 @@ struct tc358743_state {
> /* edid */
> u8 edid_blocks_written;
>
> + /* used by i2c_wr() */
> + u8 wr_data[MAX_XFER_SIZE];
> +
> struct v4l2_dv_timings timings;
> u32 mbus_fmt_code;
>
> @@ -143,9 +149,13 @@ static void i2c_wr(struct v4l2_subdev *sd, u16 reg, u8 *values, u32 n)
> {
> struct tc358743_state *state = to_state(sd);
> struct i2c_client *client = state->i2c_client;
> + u8 *data = state->wr_data;
> int err, i;
> struct i2c_msg msg;
> - u8 data[2 + n];
> +
> + if ((2 + n) > sizeof(state->wr_data))
> + v4l2_warn(sd, "i2c wr reg=%04x: len=%d is too big!\n",
> + reg, 2 + n);
>
> msg.addr = client->addr;
> msg.buf = data;
Acked-by: Mats Randgaard <matrandg@cisco.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] tc358743: don't use variable length array for I2C writes
@ 2015-08-11 15:13 Mauro Carvalho Chehab
0 siblings, 0 replies; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2015-08-11 15:13 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
drivers/media/i2c/tc358743.c:148:19: warning: Variable length array is used.
As the maximum size is 1026, we can't use dynamic var, as it
would otherwise spend 1056 bytes of the stack at i2c_wr() function.
So, allocate a buffer with the allowed maximum size together with
the state var.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
drivers/media/i2c/tc358743.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 2e926317d7e9..fe42c9a1cb78 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -59,6 +59,9 @@ MODULE_LICENSE("GPL");
#define EDID_NUM_BLOCKS_MAX 8
#define EDID_BLOCK_SIZE 128
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE (EDID_NUM_BLOCKS_MAX * EDID_BLOCK_SIZE + 2)
+
static const struct v4l2_dv_timings_cap tc358743_timings_cap = {
.type = V4L2_DV_BT_656_1120,
/* keep this initialization for compatibility with GCC < 4.4.6 */
@@ -94,6 +97,9 @@ struct tc358743_state {
/* edid */
u8 edid_blocks_written;
+ /* used by i2c_wr() */
+ u8 wr_data[MAX_XFER_SIZE];
+
struct v4l2_dv_timings timings;
u32 mbus_fmt_code;
@@ -143,9 +149,13 @@ static void i2c_wr(struct v4l2_subdev *sd, u16 reg, u8 *values, u32 n)
{
struct tc358743_state *state = to_state(sd);
struct i2c_client *client = state->i2c_client;
+ u8 *data = state->wr_data;
int err, i;
struct i2c_msg msg;
- u8 data[2 + n];
+
+ if ((2 + n) > sizeof(state->wr_data))
+ v4l2_warn(sd, "i2c wr reg=%04x: len=%d is too big!\n",
+ reg, 2 + n);
msg.addr = client->addr;
msg.buf = data;
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-08-12 12:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-11 15:18 [PATCH 1/4] tc358743: don't use variable length array for I2C writes Mauro Carvalho Chehab
2015-08-11 15:18 ` [PATCH 2/4] ov9650: remove an extra space Mauro Carvalho Chehab
2015-08-11 15:18 ` [PATCH 3/4] ov2659: get rid of unused values Mauro Carvalho Chehab
2015-08-11 15:18 ` [PATCH 4/4] sr030pc30: don't read a new pointer Mauro Carvalho Chehab
2015-08-12 12:24 ` [PATCH 1/4] tc358743: don't use variable length array for I2C writes Mats Randgaard (matrandg)
-- strict thread matches above, loose matches on Subject: below --
2015-08-11 15:13 Mauro Carvalho Chehab
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.