* [PATCH 0/9] Some smatch fixups
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
2015-06-05 14:43 ` Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 01/11] [media] drxk: better handle errors Mauro Carvalho Chehab
` (10 subsequent siblings)
11 siblings, 1 reply; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
Fix several smatch warnings.
There are still 27 smatch warnings at drivers/media:
This one:
drivers/media/pci/cx23885/cx23885-dvb.c:2046 dvb_register() Function too hairy. Giving up.
It is just to a random memory limit at smatch that allows it to
allocate only 50Mb of memory for name allocation. I fixed it
locally and submitted a fix to Dan.
Those seem to be false-positives:
drivers/media/dvb-frontends/stv0900_core.c:1183 stv0900_get_optim_carr_loop() error: buffer overflow 'cllas2' 11 <= 13
drivers/media/dvb-frontends/stv0900_core.c:1185 stv0900_get_optim_carr_loop() error: buffer overflow 'cllas2' 11 <= 13
drivers/media/dvb-frontends/stv0900_core.c:1187 stv0900_get_optim_carr_loop() error: buffer overflow 'cllas2' 11 <= 13
drivers/media/dvb-frontends/stv0900_core.c:1189 stv0900_get_optim_carr_loop() error: buffer overflow 'cllas2' 11 <= 13
drivers/media/dvb-frontends/stv0900_core.c:1191 stv0900_get_optim_carr_loop() error: buffer overflow 'cllas2' 11 <= 13
drivers/media/media-entity.c:238:17: warning: Variable length array is used.
drivers/media/media-entity.c:239:17: warning: Variable length array is used.
drivers/media/pci/ttpci/av7110.c:2210 frontend_init() warn: missing break? reassigning 'av7110->fe'
drivers/media/pci/ttpci/budget.c:631 frontend_init() warn: missing break? reassigning 'budget->dvb_frontend'
drivers/media/platform/vivid/vivid-rds-gen.c:82 vivid_rds_generate() error: buffer overflow 'rds->psname' 9 <= 43
drivers/media/platform/vivid/vivid-rds-gen.c:83 vivid_rds_generate() error: buffer overflow 'rds->psname' 9 <= 42
drivers/media/platform/vivid/vivid-rds-gen.c:89 vivid_rds_generate() error: buffer overflow 'rds->radiotext' 65 <= 84
drivers/media/platform/vivid/vivid-rds-gen.c:90 vivid_rds_generate() error: buffer overflow 'rds->radiotext' 65 <= 85
drivers/media/platform/vivid/vivid-rds-gen.c:92 vivid_rds_generate() error: buffer overflow 'rds->radiotext' 65 <= 86
drivers/media/platform/vivid/vivid-rds-gen.c:93 vivid_rds_generate() error: buffer overflow 'rds->radiotext' 65 <= 87
drivers/media/radio/radio-aimslab.c:73 rtrack_alloc() warn: possible memory leak of 'rt'
drivers/media/radio/radio-aztech.c:87 aztech_alloc() warn: possible memory leak of 'az'
drivers/media/radio/radio-gemtek.c:189 gemtek_alloc() warn: possible memory leak of 'gt'
drivers/media/radio/radio-trust.c:60 trust_alloc() warn: possible memory leak of 'tr'
drivers/media/radio/radio-typhoon.c:79 typhoon_alloc() warn: possible memory leak of 'ty'
drivers/media/radio/radio-zoltrix.c:83 zoltrix_alloc() warn: possible memory leak of 'zol'
drivers/media/usb/pvrusb2/pvrusb2-encoder.c:227 pvr2_encoder_cmd() error: buffer overflow 'wrData' 16 <= 16
drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3676 pvr2_send_request_ex() error: we previously assumed 'write_data' could be null (see line 3648)
drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3829 pvr2_send_request_ex() error: we previously assumed 'read_data' could be null (see line 3649)
I didn't find an easy/worth way to remove the above.
This one is due to a code that got commented:
drivers/media/usb/usbvision/usbvision-video.c:1072 usbvision_read() warn: inconsistent indenting
Probably the best here is to remove the commented code and fix
identation.
This one seems a real bug, but fixing it would require some tests with
the hardware, and a better understanding on what the function should be
expecting to do when steal is NULL:
drivers/media/pci/ivtv/ivtv-queue.c:145 ivtv_queue_move() error: we previously assumed 'steal' could be null (see line 138)
Mauro Carvalho Chehab (9):
[media] drxk: better handle errors
[media] em28xx: remove dead code
[media] sh_vou: avoid going past arrays
dib0090: Remove a dead code
[media] bt8xx: remove needless check
[media] ivtv: fix two smatch warnings
tm6000: remove needless check
[media] ir: Fix IR_MAX_DURATION enforcement
rc: set IR_MAX_DURATION to 500 ms
drivers/media/dvb-frontends/dib0090.c | 4 +-
drivers/media/dvb-frontends/drxk_hard.c | 7 +-
drivers/media/pci/bt8xx/dst_ca.c | 132 ++++++++++++++++----------------
drivers/media/pci/ivtv/ivtv-driver.h | 3 +-
drivers/media/platform/sh_vou.c | 14 ++--
drivers/media/rc/redrat3.c | 5 +-
drivers/media/rc/streamzap.c | 6 +-
drivers/media/usb/em28xx/em28xx-video.c | 1 -
drivers/media/usb/tm6000/tm6000-video.c | 2 +-
include/media/rc-core.h | 2 +-
10 files changed, 87 insertions(+), 89 deletions(-)
--
2.4.2
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 0/9] Some smatch fixups
2015-06-05 14:27 ` [PATCH 0/9] " Mauro Carvalho Chehab
@ 2015-06-05 14:43 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:43 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab
Em Fri, 05 Jun 2015 11:27:33 -0300
Mauro Carvalho Chehab <mchehab@osg.samsung.com> escreveu:
> Fix several smatch warnings.
>
> There are still 27 smatch warnings at drivers/media:
Please discard this e-mail. Git sent it by accident. This is a version
of patch 0/11, saved with another name (and another extension).
The same applies to the other e-mail with the very same subject.
The real one is "[PATCH 00/11] Some smatch fixups".
>
>
> This one:
> drivers/media/pci/cx23885/cx23885-dvb.c:2046 dvb_register() Function too hairy. Giving up.
>
> It is just to a random memory limit at smatch that allows it to
> allocate only 50Mb of memory for name allocation. I fixed it
> locally and submitted a fix to Dan.
>
> Those seem to be false-positives:
> drivers/media/dvb-frontends/stv0900_core.c:1183 stv0900_get_optim_carr_loop() error: buffer overflow 'cllas2' 11 <= 13
> drivers/media/dvb-frontends/stv0900_core.c:1185 stv0900_get_optim_carr_loop() error: buffer overflow 'cllas2' 11 <= 13
> drivers/media/dvb-frontends/stv0900_core.c:1187 stv0900_get_optim_carr_loop() error: buffer overflow 'cllas2' 11 <= 13
> drivers/media/dvb-frontends/stv0900_core.c:1189 stv0900_get_optim_carr_loop() error: buffer overflow 'cllas2' 11 <= 13
> drivers/media/dvb-frontends/stv0900_core.c:1191 stv0900_get_optim_carr_loop() error: buffer overflow 'cllas2' 11 <= 13
> drivers/media/media-entity.c:238:17: warning: Variable length array is used.
> drivers/media/media-entity.c:239:17: warning: Variable length array is used.
> drivers/media/pci/ttpci/av7110.c:2210 frontend_init() warn: missing break? reassigning 'av7110->fe'
> drivers/media/pci/ttpci/budget.c:631 frontend_init() warn: missing break? reassigning 'budget->dvb_frontend'
> drivers/media/platform/vivid/vivid-rds-gen.c:82 vivid_rds_generate() error: buffer overflow 'rds->psname' 9 <= 43
> drivers/media/platform/vivid/vivid-rds-gen.c:83 vivid_rds_generate() error: buffer overflow 'rds->psname' 9 <= 42
> drivers/media/platform/vivid/vivid-rds-gen.c:89 vivid_rds_generate() error: buffer overflow 'rds->radiotext' 65 <= 84
> drivers/media/platform/vivid/vivid-rds-gen.c:90 vivid_rds_generate() error: buffer overflow 'rds->radiotext' 65 <= 85
> drivers/media/platform/vivid/vivid-rds-gen.c:92 vivid_rds_generate() error: buffer overflow 'rds->radiotext' 65 <= 86
> drivers/media/platform/vivid/vivid-rds-gen.c:93 vivid_rds_generate() error: buffer overflow 'rds->radiotext' 65 <= 87
> drivers/media/radio/radio-aimslab.c:73 rtrack_alloc() warn: possible memory leak of 'rt'
> drivers/media/radio/radio-aztech.c:87 aztech_alloc() warn: possible memory leak of 'az'
> drivers/media/radio/radio-gemtek.c:189 gemtek_alloc() warn: possible memory leak of 'gt'
> drivers/media/radio/radio-trust.c:60 trust_alloc() warn: possible memory leak of 'tr'
> drivers/media/radio/radio-typhoon.c:79 typhoon_alloc() warn: possible memory leak of 'ty'
> drivers/media/radio/radio-zoltrix.c:83 zoltrix_alloc() warn: possible memory leak of 'zol'
> drivers/media/usb/pvrusb2/pvrusb2-encoder.c:227 pvr2_encoder_cmd() error: buffer overflow 'wrData' 16 <= 16
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3676 pvr2_send_request_ex() error: we previously assumed 'write_data' could be null (see line 3648)
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3829 pvr2_send_request_ex() error: we previously assumed 'read_data' could be null (see line 3649)
>
> I didn't find an easy/worth way to remove the above.
>
> This one is due to a code that got commented:
> drivers/media/usb/usbvision/usbvision-video.c:1072 usbvision_read() warn: inconsistent indenting
>
> Probably the best here is to remove the commented code and fix
> identation.
>
> This one seems a real bug, but fixing it would require some tests with
> the hardware, and a better understanding on what the function should be
> expecting to do when steal is NULL:
> drivers/media/pci/ivtv/ivtv-queue.c:145 ivtv_queue_move() error: we previously assumed 'steal' could be null (see line 138)
>
> Mauro Carvalho Chehab (9):
> [media] drxk: better handle errors
> [media] em28xx: remove dead code
> [media] sh_vou: avoid going past arrays
> dib0090: Remove a dead code
> [media] bt8xx: remove needless check
> [media] ivtv: fix two smatch warnings
> tm6000: remove needless check
> [media] ir: Fix IR_MAX_DURATION enforcement
> rc: set IR_MAX_DURATION to 500 ms
>
> drivers/media/dvb-frontends/dib0090.c | 4 +-
> drivers/media/dvb-frontends/drxk_hard.c | 7 +-
> drivers/media/pci/bt8xx/dst_ca.c | 132 ++++++++++++++++----------------
> drivers/media/pci/ivtv/ivtv-driver.h | 3 +-
> drivers/media/platform/sh_vou.c | 14 ++--
> drivers/media/rc/redrat3.c | 5 +-
> drivers/media/rc/streamzap.c | 6 +-
> drivers/media/usb/em28xx/em28xx-video.c | 1 -
> drivers/media/usb/tm6000/tm6000-video.c | 2 +-
> include/media/rc-core.h | 2 +-
> 10 files changed, 87 insertions(+), 89 deletions(-)
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 01/11] [media] drxk: better handle errors
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 0/9] " Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 02/11] [media] em28xx: remove dead code Mauro Carvalho Chehab
` (9 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Markus Elfring
As reported by smatch:
drivers/media/dvb-frontends/drxk_hard.c:3277 dvbt_sc_command() warn: missing break? reassigning 'status'
This is basically because the error handling logic there was crappy.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/dvb-frontends/drxk_hard.c b/drivers/media/dvb-frontends/drxk_hard.c
index ad35264a3819..b1fc4bd44a2b 100644
--- a/drivers/media/dvb-frontends/drxk_hard.c
+++ b/drivers/media/dvb-frontends/drxk_hard.c
@@ -3262,6 +3262,7 @@ static int dvbt_sc_command(struct drxk_state *state,
}
/* Write needed parameters and the command */
+ status = 0;
switch (cmd) {
/* All commands using 5 parameters */
/* All commands using 4 parameters */
@@ -3270,16 +3271,16 @@ static int dvbt_sc_command(struct drxk_state *state,
case OFDM_SC_RA_RAM_CMD_PROC_START:
case OFDM_SC_RA_RAM_CMD_SET_PREF_PARAM:
case OFDM_SC_RA_RAM_CMD_PROGRAM_PARAM:
- status = write16(state, OFDM_SC_RA_RAM_PARAM1__A, param1);
+ status |= write16(state, OFDM_SC_RA_RAM_PARAM1__A, param1);
/* All commands using 1 parameters */
case OFDM_SC_RA_RAM_CMD_SET_ECHO_TIMING:
case OFDM_SC_RA_RAM_CMD_USER_IO:
- status = write16(state, OFDM_SC_RA_RAM_PARAM0__A, param0);
+ status |= write16(state, OFDM_SC_RA_RAM_PARAM0__A, param0);
/* All commands using 0 parameters */
case OFDM_SC_RA_RAM_CMD_GET_OP_PARAM:
case OFDM_SC_RA_RAM_CMD_NULL:
/* Write command */
- status = write16(state, OFDM_SC_RA_RAM_CMD__A, cmd);
+ status |= write16(state, OFDM_SC_RA_RAM_CMD__A, cmd);
break;
default:
/* Unknown command */
--
2.4.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 02/11] [media] em28xx: remove dead code
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 0/9] " Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 01/11] [media] drxk: better handle errors Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 03/11] [media] sh_vou: avoid going past arrays Mauro Carvalho Chehab
` (8 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
As reported by smatch:
drivers/media/usb/em28xx/em28xx-video.c:842 get_ressource() info: ignoring unreachable code.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 14eba9c65de3..4397ce5e78df 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -839,7 +839,6 @@ static int get_ressource(enum v4l2_buf_type f_type)
return EM28XX_RESOURCE_VBI;
default:
BUG();
- return 0;
}
}
--
2.4.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 03/11] [media] sh_vou: avoid going past arrays
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
` (2 preceding siblings ...)
2015-06-05 14:27 ` [PATCH 02/11] [media] em28xx: remove dead code Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 04/11] [media] dib0090: Remove a dead code Mauro Carvalho Chehab
` (7 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Hans Verkuil,
Prabhakar Lad, Wolfram Sang, Boris BREZILLON, Alexey Khoroshilov
Smatch reports two issues:
drivers/media/platform/sh_vou.c:670 vou_adjust_output() error: buffer overflow 'vou_scale_v_num' 3 <= 4
drivers/media/platform/sh_vou.c:670 vou_adjust_output() error: buffer overflow 'vou_scale_v_den' 3 <= 4
It seems that there's actually a bug here: the same var (idx) is used
as an index for vertical and horizontal scaling arrays. However,
there are 4 elements on the h arrays, and only 3 at the v ones.
On the first loop, it may select index 4 for the horizontal array.
In this case, if the second loop fails to select an index, the
code would keep using 4 for the vertical array, with is past of
the array sizes.
The intent here seems to use index 0, if the scale is not found.
So, use a separate var for the vertical index.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/platform/sh_vou.c b/drivers/media/platform/sh_vou.c
index 829e85c26610..8b799bae01b8 100644
--- a/drivers/media/platform/sh_vou.c
+++ b/drivers/media/platform/sh_vou.c
@@ -600,7 +600,7 @@ static void vou_adjust_output(struct sh_vou_geometry *geo, v4l2_std_id std)
{
unsigned int best_err = UINT_MAX, best = geo->in_width,
width_max, height_max, img_height_max;
- int i, idx = 0;
+ int i, idx_h = 0, idx_v = 0;
if (std & V4L2_STD_525_60) {
width_max = 858;
@@ -625,7 +625,7 @@ static void vou_adjust_output(struct sh_vou_geometry *geo, v4l2_std_id std)
err = abs(found - geo->output.width);
if (err < best_err) {
best_err = err;
- idx = i;
+ idx_h = i;
best = found;
}
if (!err)
@@ -633,12 +633,12 @@ static void vou_adjust_output(struct sh_vou_geometry *geo, v4l2_std_id std)
}
geo->output.width = best;
- geo->scale_idx_h = idx;
+ geo->scale_idx_h = idx_h;
if (geo->output.left + best > width_max)
geo->output.left = width_max - best;
pr_debug("%s(): W %u * %u/%u = %u\n", __func__, geo->in_width,
- vou_scale_h_num[idx], vou_scale_h_den[idx], best);
+ vou_scale_h_num[idx_h], vou_scale_h_den[idx_h], best);
best_err = UINT_MAX;
@@ -655,7 +655,7 @@ static void vou_adjust_output(struct sh_vou_geometry *geo, v4l2_std_id std)
err = abs(found - geo->output.height);
if (err < best_err) {
best_err = err;
- idx = i;
+ idx_v = i;
best = found;
}
if (!err)
@@ -663,12 +663,12 @@ static void vou_adjust_output(struct sh_vou_geometry *geo, v4l2_std_id std)
}
geo->output.height = best;
- geo->scale_idx_v = idx;
+ geo->scale_idx_v = idx_v;
if (geo->output.top + best > height_max)
geo->output.top = height_max - best;
pr_debug("%s(): H %u * %u/%u = %u\n", __func__, geo->in_height,
- vou_scale_v_num[idx], vou_scale_v_den[idx], best);
+ vou_scale_v_num[idx_v], vou_scale_v_den[idx_v], best);
}
static int sh_vou_s_fmt_vid_out(struct file *file, void *priv,
--
2.4.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 04/11] [media] dib0090: Remove a dead code
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
` (3 preceding siblings ...)
2015-06-05 14:27 ` [PATCH 03/11] [media] sh_vou: avoid going past arrays Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 05/11] [media] bt8xx: remove needless check Mauro Carvalho Chehab
` (6 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
As reported by smatch:
drivers/media/dvb-frontends/dib0090.c:1710 dib0090_dc_offset_calibration() warn: missing break? reassigning '*tune_state'
There's no need to change tune_state there, as the fall though code
will change it again to another state. So, simplify it by
removing the dead code.
While here, fix a typo:
Sart => Start
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/dvb-frontends/dib0090.c b/drivers/media/dvb-frontends/dib0090.c
index 68e2af2650d3..47cb72243b9d 100644
--- a/drivers/media/dvb-frontends/dib0090.c
+++ b/drivers/media/dvb-frontends/dib0090.c
@@ -1696,12 +1696,10 @@ static int dib0090_dc_offset_calibration(struct dib0090_state *state, enum front
if (state->identity.p1g)
state->dc = dc_p1g_table;
- *tune_state = CT_TUNER_STEP_0;
/* fall through */
-
case CT_TUNER_STEP_0:
- dprintk("Sart/continue DC calibration for %s path", (state->dc->i == 1) ? "I" : "Q");
+ dprintk("Start/continue DC calibration for %s path", (state->dc->i == 1) ? "I" : "Q");
dib0090_write_reg(state, 0x01, state->dc->bb1);
dib0090_write_reg(state, 0x07, state->bb7 | (state->dc->i << 7));
--
2.4.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 05/11] [media] bt8xx: remove needless check
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
` (4 preceding siblings ...)
2015-06-05 14:27 ` [PATCH 04/11] [media] dib0090: Remove a dead code Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 06/11] [media] ivtv: fix two smatch warnings Mauro Carvalho Chehab
` (5 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
As reported by smatch:
drivers/media/pci/bt8xx/dst_ca.c:323 ca_get_message() warn: this array is probably non-NULL. 'p_ca_message->msg'
drivers/media/pci/bt8xx/dst_ca.c:498 ca_send_message() warn: this array is probably non-NULL. 'p_ca_message->msg'
Those two checks are needless/useless, as the ca_msg struct is
declared as:
typedef struct ca_msg {
unsigned int index;
unsigned int type;
unsigned int length;
unsigned char msg[256];
} ca_msg_t;
So, if the p_ca_message pointer is not null, msg will also be
not null.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/pci/bt8xx/dst_ca.c b/drivers/media/pci/bt8xx/dst_ca.c
index c22c4ae06844..c5cc14ef8347 100644
--- a/drivers/media/pci/bt8xx/dst_ca.c
+++ b/drivers/media/pci/bt8xx/dst_ca.c
@@ -320,29 +320,27 @@ static int ca_get_message(struct dst_state *state, struct ca_msg *p_ca_message,
if (copy_from_user(p_ca_message, arg, sizeof (struct ca_msg)))
return -EFAULT;
- if (p_ca_message->msg) {
- dprintk(verbose, DST_CA_NOTICE, 1, " Message = [%*ph]",
- 3, p_ca_message->msg);
+ dprintk(verbose, DST_CA_NOTICE, 1, " Message = [%*ph]",
+ 3, p_ca_message->msg);
- for (i = 0; i < 3; i++) {
- command = command | p_ca_message->msg[i];
- if (i < 2)
- command = command << 8;
- }
- dprintk(verbose, DST_CA_NOTICE, 1, " Command=[0x%x]", command);
+ for (i = 0; i < 3; i++) {
+ command = command | p_ca_message->msg[i];
+ if (i < 2)
+ command = command << 8;
+ }
+ dprintk(verbose, DST_CA_NOTICE, 1, " Command=[0x%x]", command);
- switch (command) {
- case CA_APP_INFO:
- memcpy(p_ca_message->msg, state->messages, 128);
- if (copy_to_user(arg, p_ca_message, sizeof (struct ca_msg)) )
- return -EFAULT;
- break;
- case CA_INFO:
- memcpy(p_ca_message->msg, state->messages, 128);
- if (copy_to_user(arg, p_ca_message, sizeof (struct ca_msg)) )
- return -EFAULT;
- break;
- }
+ switch (command) {
+ case CA_APP_INFO:
+ memcpy(p_ca_message->msg, state->messages, 128);
+ if (copy_to_user(arg, p_ca_message, sizeof (struct ca_msg)) )
+ return -EFAULT;
+ break;
+ case CA_INFO:
+ memcpy(p_ca_message->msg, state->messages, 128);
+ if (copy_to_user(arg, p_ca_message, sizeof (struct ca_msg)) )
+ return -EFAULT;
+ break;
}
return 0;
@@ -494,60 +492,58 @@ static int ca_send_message(struct dst_state *state, struct ca_msg *p_ca_message,
goto free_mem_and_exit;
}
+ /* EN50221 tag */
+ command = 0;
- if (p_ca_message->msg) {
- /* EN50221 tag */
- command = 0;
+ for (i = 0; i < 3; i++) {
+ command = command | p_ca_message->msg[i];
+ if (i < 2)
+ command = command << 8;
+ }
+ dprintk(verbose, DST_CA_DEBUG, 1, " Command=[0x%x]\n", command);
- for (i = 0; i < 3; i++) {
- command = command | p_ca_message->msg[i];
- if (i < 2)
- command = command << 8;
+ switch (command) {
+ case CA_PMT:
+ dprintk(verbose, DST_CA_DEBUG, 1, "Command = SEND_CA_PMT");
+ if ((ca_set_pmt(state, p_ca_message, hw_buffer, 0, 0)) < 0) { // code simplification started
+ dprintk(verbose, DST_CA_ERROR, 1, " -->CA_PMT Failed !");
+ result = -1;
+ goto free_mem_and_exit;
}
- dprintk(verbose, DST_CA_DEBUG, 1, " Command=[0x%x]\n", command);
-
- switch (command) {
- case CA_PMT:
- dprintk(verbose, DST_CA_DEBUG, 1, "Command = SEND_CA_PMT");
- if ((ca_set_pmt(state, p_ca_message, hw_buffer, 0, 0)) < 0) { // code simplification started
- dprintk(verbose, DST_CA_ERROR, 1, " -->CA_PMT Failed !");
- result = -1;
- goto free_mem_and_exit;
- }
- dprintk(verbose, DST_CA_INFO, 1, " -->CA_PMT Success !");
- break;
- case CA_PMT_REPLY:
- dprintk(verbose, DST_CA_INFO, 1, "Command = CA_PMT_REPLY");
- /* Have to handle the 2 basic types of cards here */
- if ((dst_check_ca_pmt(state, p_ca_message, hw_buffer)) < 0) {
- dprintk(verbose, DST_CA_ERROR, 1, " -->CA_PMT_REPLY Failed !");
- result = -1;
- goto free_mem_and_exit;
- }
- dprintk(verbose, DST_CA_INFO, 1, " -->CA_PMT_REPLY Success !");
- break;
- case CA_APP_INFO_ENQUIRY: // only for debugging
- dprintk(verbose, DST_CA_INFO, 1, " Getting Cam Application information");
+ dprintk(verbose, DST_CA_INFO, 1, " -->CA_PMT Success !");
+ break;
+ case CA_PMT_REPLY:
+ dprintk(verbose, DST_CA_INFO, 1, "Command = CA_PMT_REPLY");
+ /* Have to handle the 2 basic types of cards here */
+ if ((dst_check_ca_pmt(state, p_ca_message, hw_buffer)) < 0) {
+ dprintk(verbose, DST_CA_ERROR, 1, " -->CA_PMT_REPLY Failed !");
+ result = -1;
+ goto free_mem_and_exit;
+ }
+ dprintk(verbose, DST_CA_INFO, 1, " -->CA_PMT_REPLY Success !");
+ break;
+ case CA_APP_INFO_ENQUIRY: // only for debugging
+ dprintk(verbose, DST_CA_INFO, 1, " Getting Cam Application information");
- if ((ca_get_app_info(state)) < 0) {
- dprintk(verbose, DST_CA_ERROR, 1, " -->CA_APP_INFO_ENQUIRY Failed !");
- result = -1;
- goto free_mem_and_exit;
- }
- dprintk(verbose, DST_CA_INFO, 1, " -->CA_APP_INFO_ENQUIRY Success !");
- break;
- case CA_INFO_ENQUIRY:
- dprintk(verbose, DST_CA_INFO, 1, " Getting CA Information");
+ if ((ca_get_app_info(state)) < 0) {
+ dprintk(verbose, DST_CA_ERROR, 1, " -->CA_APP_INFO_ENQUIRY Failed !");
+ result = -1;
+ goto free_mem_and_exit;
+ }
+ dprintk(verbose, DST_CA_INFO, 1, " -->CA_APP_INFO_ENQUIRY Success !");
+ break;
+ case CA_INFO_ENQUIRY:
+ dprintk(verbose, DST_CA_INFO, 1, " Getting CA Information");
- if ((ca_get_ca_info(state)) < 0) {
- dprintk(verbose, DST_CA_ERROR, 1, " -->CA_INFO_ENQUIRY Failed !");
- result = -1;
- goto free_mem_and_exit;
- }
- dprintk(verbose, DST_CA_INFO, 1, " -->CA_INFO_ENQUIRY Success !");
- break;
+ if ((ca_get_ca_info(state)) < 0) {
+ dprintk(verbose, DST_CA_ERROR, 1, " -->CA_INFO_ENQUIRY Failed !");
+ result = -1;
+ goto free_mem_and_exit;
}
+ dprintk(verbose, DST_CA_INFO, 1, " -->CA_INFO_ENQUIRY Success !");
+ break;
}
+
free_mem_and_exit:
kfree (hw_buffer);
--
2.4.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 06/11] [media] ivtv: fix two smatch warnings
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
` (5 preceding siblings ...)
2015-06-05 14:27 ` [PATCH 05/11] [media] bt8xx: remove needless check Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 07/11] [media] tm6000: remove needless check Mauro Carvalho Chehab
` (4 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Andy Walls
Smatch currently produces two warnings:
drivers/media/pci/ivtv/ivtv-fileops.c:901 ivtv_v4l2_close() warn: suspicious bitop condition
drivers/media/pci/ivtv/ivtv-fileops.c:1026 ivtv_open() warn: suspicious bitop condition
Those are false positives, but it is not hard to get rid of them by
using a different way to evaluate the macro, splitting the logical
boolean evaluation from the bitmap one.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/pci/ivtv/ivtv-driver.h b/drivers/media/pci/ivtv/ivtv-driver.h
index e8b6c7ad2ba9..ee0ef6e48c7d 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.h
+++ b/drivers/media/pci/ivtv/ivtv-driver.h
@@ -830,7 +830,8 @@ static inline int ivtv_raw_vbi(const struct ivtv *itv)
do { \
struct v4l2_subdev *__sd; \
__v4l2_device_call_subdevs_p(&(itv)->v4l2_dev, __sd, \
- !(hw) || (__sd->grp_id & (hw)), o, f , ##args); \
+ !(hw) ? true : (__sd->grp_id & (hw)), \
+ o, f, ##args); \
} while (0)
#define ivtv_call_all(itv, o, f, args...) ivtv_call_hw(itv, 0, o, f , ##args)
--
2.4.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 07/11] [media] tm6000: remove needless check
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
` (6 preceding siblings ...)
2015-06-05 14:27 ` [PATCH 06/11] [media] ivtv: fix two smatch warnings Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 08/11] [media] ir: Fix IR_MAX_DURATION enforcement Mauro Carvalho Chehab
` (3 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Hans Verkuil,
Ramakrishnan Muthukrishnan, Sakari Ailus, Laurent Pinchart
Smatch reports a warning:
drivers/media/usb/tm6000/tm6000-video.c:646 tm6000_prepare_isoc() error: we previously assumed 'dev->urb_buffer' could be null (see line 624)
This is not really a problem, but it actually shows that the check
if urb_buffer is NULL is being done twice: at the if and at
tm6000_alloc_urb_buffers().
We don't need to do it twice. So, remove the extra check. The code
become cleaner, and, as a collateral effect, smatch becomes happy.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/usb/tm6000/tm6000-video.c b/drivers/media/usb/tm6000/tm6000-video.c
index 77ce9efe1f24..5287d2960282 100644
--- a/drivers/media/usb/tm6000/tm6000-video.c
+++ b/drivers/media/usb/tm6000/tm6000-video.c
@@ -621,7 +621,7 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev)
dev->isoc_in.maxsize, size);
- if (!dev->urb_buffer && tm6000_alloc_urb_buffers(dev) < 0) {
+ if (tm6000_alloc_urb_buffers(dev) < 0) {
tm6000_err("cannot allocate memory for urb buffers\n");
/* call free, as some buffers might have been allocated */
--
2.4.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 08/11] [media] ir: Fix IR_MAX_DURATION enforcement
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
` (7 preceding siblings ...)
2015-06-05 14:27 ` [PATCH 07/11] [media] tm6000: remove needless check Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
2015-06-05 14:55 ` Sean Young
2015-06-05 14:27 ` [PATCH 09/11] [media] rc: set IR_MAX_DURATION to 500 ms Mauro Carvalho Chehab
` (2 subsequent siblings)
11 siblings, 1 reply; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Sean Young,
David Härdeman, Himangi Saraogi, Julia Lawall
Don't assume that IR_MAX_DURATION is a bitmask. It isn't.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index c83292ad1b34..ec74244a3853 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -322,7 +322,7 @@ static u32 redrat3_us_to_len(u32 microsec)
u32 result;
u32 divisor;
- microsec &= IR_MAX_DURATION;
+ microsec = (microsec > IR_MAX_DURATION) ? IR_MAX_DURATION : microsec;
divisor = (RR3_CLK_CONV_FACTOR / 1000);
result = (u32)(microsec * divisor) / 1000;
@@ -380,7 +380,8 @@ static void redrat3_process_ir_data(struct redrat3_dev *rr3)
if (i == 0)
trailer = rawir.duration;
/* cap the value to IR_MAX_DURATION */
- rawir.duration &= IR_MAX_DURATION;
+ rawir.duration = (rawir.duration > IR_MAX_DURATION) ?
+ IR_MAX_DURATION : rawir.duration;
dev_dbg(dev, "storing %s with duration %d (i: %d)\n",
rawir.pulse ? "pulse" : "space", rawir.duration, i);
diff --git a/drivers/media/rc/streamzap.c b/drivers/media/rc/streamzap.c
index bf4a44272f0e..5a17cb88ff27 100644
--- a/drivers/media/rc/streamzap.c
+++ b/drivers/media/rc/streamzap.c
@@ -152,7 +152,8 @@ static void sz_push_full_pulse(struct streamzap_ir *sz,
sz->signal_last.tv_usec);
rawir.duration -= sz->sum;
rawir.duration = US_TO_NS(rawir.duration);
- rawir.duration &= IR_MAX_DURATION;
+ rawir.duration = (rawir.duration > IR_MAX_DURATION) ?
+ IR_MAX_DURATION : rawir.duration;
}
sz_push(sz, rawir);
@@ -165,7 +166,8 @@ static void sz_push_full_pulse(struct streamzap_ir *sz,
rawir.duration += SZ_RESOLUTION / 2;
sz->sum += rawir.duration;
rawir.duration = US_TO_NS(rawir.duration);
- rawir.duration &= IR_MAX_DURATION;
+ rawir.duration = (rawir.duration > IR_MAX_DURATION) ?
+ IR_MAX_DURATION : rawir.duration;
sz_push(sz, rawir);
}
--
2.4.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 08/11] [media] ir: Fix IR_MAX_DURATION enforcement
2015-06-05 14:27 ` [PATCH 08/11] [media] ir: Fix IR_MAX_DURATION enforcement Mauro Carvalho Chehab
@ 2015-06-05 14:55 ` Sean Young
2015-06-05 15:00 ` Sean Young
0 siblings, 1 reply; 17+ messages in thread
From: Sean Young @ 2015-06-05 14:55 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
David Härdeman, Himangi Saraogi, Julia Lawall
On Fri, Jun 05, 2015 at 11:27:41AM -0300, Mauro Carvalho Chehab wrote:
> Don't assume that IR_MAX_DURATION is a bitmask. It isn't.
The patch is right, but note that IR_MAX_DURATION is 0xffffffff, and in
all these cases it is being compared to a u32, so it is always false.
Should these statements simply be removed? None of the other drivers
do these checks.
Sean
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
> index c83292ad1b34..ec74244a3853 100644
> --- a/drivers/media/rc/redrat3.c
> +++ b/drivers/media/rc/redrat3.c
> @@ -322,7 +322,7 @@ static u32 redrat3_us_to_len(u32 microsec)
> u32 result;
> u32 divisor;
>
> - microsec &= IR_MAX_DURATION;
> + microsec = (microsec > IR_MAX_DURATION) ? IR_MAX_DURATION : microsec;
> divisor = (RR3_CLK_CONV_FACTOR / 1000);
> result = (u32)(microsec * divisor) / 1000;
>
> @@ -380,7 +380,8 @@ static void redrat3_process_ir_data(struct redrat3_dev *rr3)
> if (i == 0)
> trailer = rawir.duration;
> /* cap the value to IR_MAX_DURATION */
> - rawir.duration &= IR_MAX_DURATION;
> + rawir.duration = (rawir.duration > IR_MAX_DURATION) ?
> + IR_MAX_DURATION : rawir.duration;
>
> dev_dbg(dev, "storing %s with duration %d (i: %d)\n",
> rawir.pulse ? "pulse" : "space", rawir.duration, i);
> diff --git a/drivers/media/rc/streamzap.c b/drivers/media/rc/streamzap.c
> index bf4a44272f0e..5a17cb88ff27 100644
> --- a/drivers/media/rc/streamzap.c
> +++ b/drivers/media/rc/streamzap.c
> @@ -152,7 +152,8 @@ static void sz_push_full_pulse(struct streamzap_ir *sz,
> sz->signal_last.tv_usec);
> rawir.duration -= sz->sum;
> rawir.duration = US_TO_NS(rawir.duration);
> - rawir.duration &= IR_MAX_DURATION;
> + rawir.duration = (rawir.duration > IR_MAX_DURATION) ?
> + IR_MAX_DURATION : rawir.duration;
> }
> sz_push(sz, rawir);
>
> @@ -165,7 +166,8 @@ static void sz_push_full_pulse(struct streamzap_ir *sz,
> rawir.duration += SZ_RESOLUTION / 2;
> sz->sum += rawir.duration;
> rawir.duration = US_TO_NS(rawir.duration);
> - rawir.duration &= IR_MAX_DURATION;
> + rawir.duration = (rawir.duration > IR_MAX_DURATION) ?
> + IR_MAX_DURATION : rawir.duration;
> sz_push(sz, rawir);
> }
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 08/11] [media] ir: Fix IR_MAX_DURATION enforcement
2015-06-05 14:55 ` Sean Young
@ 2015-06-05 15:00 ` Sean Young
2015-06-05 15:04 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 17+ messages in thread
From: Sean Young @ 2015-06-05 15:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
David Härdeman, Himangi Saraogi, Julia Lawall
On Fri, Jun 05, 2015 at 03:55:38PM +0100, Sean Young wrote:
> On Fri, Jun 05, 2015 at 11:27:41AM -0300, Mauro Carvalho Chehab wrote:
> > Don't assume that IR_MAX_DURATION is a bitmask. It isn't.
>
> The patch is right, but note that IR_MAX_DURATION is 0xffffffff, and in
> all these cases it is being compared to a u32, so it is always false.
>
> Should these statements simply be removed? None of the other drivers
> do these checks.
Sorry please ignore me, I should have read the whole patch series. :(
Sean
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 08/11] [media] ir: Fix IR_MAX_DURATION enforcement
2015-06-05 15:00 ` Sean Young
@ 2015-06-05 15:04 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 15:04 UTC (permalink / raw)
To: Sean Young
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
David Härdeman, Himangi Saraogi, Julia Lawall
Em Fri, 05 Jun 2015 16:00:43 +0100
Sean Young <sean@mess.org> escreveu:
> On Fri, Jun 05, 2015 at 03:55:38PM +0100, Sean Young wrote:
> > On Fri, Jun 05, 2015 at 11:27:41AM -0300, Mauro Carvalho Chehab wrote:
> > > Don't assume that IR_MAX_DURATION is a bitmask. It isn't.
> >
> > The patch is right, but note that IR_MAX_DURATION is 0xffffffff, and in
> > all these cases it is being compared to a u32, so it is always false.
> >
> > Should these statements simply be removed? None of the other drivers
> > do these checks.
>
> Sorry please ignore me, I should have read the whole patch series. :(
Yeah, patch 9/11 addresses it. We'll very likely need a check against
a maximum value. The Y2038 patches converting several timestamps to 64 bits.
Regards,
Mauro
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 09/11] [media] rc: set IR_MAX_DURATION to 500 ms
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
` (8 preceding siblings ...)
2015-06-05 14:27 ` [PATCH 08/11] [media] ir: Fix IR_MAX_DURATION enforcement Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 10/11] [media] usbvision: cleanup the code Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 11/11] [media] lirc_imon: simplify error handling code Mauro Carvalho Chehab
11 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab
The current definition is weird, and produce lots of sparse
warnings:
drivers/media/i2c/cx25840/cx25840-ir.c:448 txclk_tx_s_max_pulse_width() warn: impossible condition '(ns > 4294967295) => (0-u32max > u32max)'
drivers/media/i2c/cx25840/cx25840-ir.c:461 rxclk_rx_s_max_pulse_width() warn: impossible condition '(ns > 4294967295) => (0-u32max > u32max)'
drivers/media/i2c/cx25840/cx25840-ir.c:706 cx25840_ir_rx_read() warn: impossible condition '(v > 4294967295) => (0-u32max > u32max)'
drivers/media/pci/ivtv/ivtv-queue.c:145 ivtv_queue_move() error: we previously assumed 'steal' could be null (see line 138)
drivers/media/rc/streamzap.c:155 sz_push_full_pulse() warn: impossible condition '(rawir.duration > 4294967295) => (0-u32max > u32max)'
drivers/media/rc/streamzap.c:169 sz_push_full_pulse() warn: impossible condition '(rawir.duration > 4294967295) => (0-u32max > u32max)'
drivers/media/rc/redrat3.c:325 redrat3_us_to_len() warn: impossible condition '(microsec > 4294967295) => (0-u32max > u32max)'
drivers/media/rc/redrat3.c:383 redrat3_process_ir_data() warn: impossible condition '(rawir.duration > 4294967295) => (0-u32max > u32max)'
drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3676 pvr2_send_request_ex() error: we previously assumed 'write_data' could be null (see line 3648)
drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3829 pvr2_send_request_ex() error: we previously assumed 'read_data' could be null (see line 3649)
drivers/media/pci/cx23885/cx23888-ir.c:463 txclk_tx_s_max_pulse_width() warn: impossible condition '(ns > 4294967295) => (0-u32max > u32max)'
drivers/media/pci/cx23885/cx23888-ir.c:476 rxclk_rx_s_max_pulse_width() warn: impossible condition '(ns > 4294967295) => (0-u32max > u32max)'
drivers/media/pci/cx23885/cx23888-ir.c:696 cx23888_ir_rx_read() warn: impossible condition '(v > 4294967295) => (0-u32max > u32max)'
Use a more realistic value for it.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index f1cb9daba489..45534da57759 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -242,7 +242,7 @@ static inline void init_ir_raw_event(struct ir_raw_event *ev)
memset(ev, 0, sizeof(*ev));
}
-#define IR_MAX_DURATION 0xFFFFFFFF /* a bit more than 4 seconds */
+#define IR_MAX_DURATION 500000000 /* 500 ms */
#define US_TO_NS(usec) ((usec) * 1000)
#define MS_TO_US(msec) ((msec) * 1000)
#define MS_TO_NS(msec) ((msec) * 1000 * 1000)
--
2.4.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 10/11] [media] usbvision: cleanup the code
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
` (9 preceding siblings ...)
2015-06-05 14:27 ` [PATCH 09/11] [media] rc: set IR_MAX_DURATION to 500 ms Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
2015-06-05 14:27 ` [PATCH 11/11] [media] lirc_imon: simplify error handling code Mauro Carvalho Chehab
11 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Hans Verkuil,
Alexey Khoroshilov, Dan Carpenter, Sakari Ailus
There's a dead code on usbvision that makes it harder to read
and produces a smatch warning about bad identation.
Improve the code readability and add a FIXME to warn about
the current hack there.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/usb/usbvision/usbvision-video.c b/drivers/media/usb/usbvision/usbvision-video.c
index 12b403e78d52..1c6d31f7c1b9 100644
--- a/drivers/media/usb/usbvision/usbvision-video.c
+++ b/drivers/media/usb/usbvision/usbvision-video.c
@@ -1061,13 +1061,24 @@ static ssize_t usbvision_read(struct file *file, char __user *buf,
__func__,
(unsigned long)count, frame->bytes_read);
- /* For now, forget the frame if it has not been read in one shot. */
-/* if (frame->bytes_read >= frame->scanlength) {*/ /* All data has been read */
+#if 1
+ /*
+ * FIXME:
+ * For now, forget the frame if it has not been read in one shot.
+ */
+ frame->bytes_read = 0;
+
+ /* Mark it as available to be used again. */
+ frame->grabstate = frame_state_unused;
+#else
+ if (frame->bytes_read >= frame->scanlength) {
+ /* All data has been read */
frame->bytes_read = 0;
/* Mark it as available to be used again. */
frame->grabstate = frame_state_unused;
-/* } */
+ }
+#endif
return count;
}
--
2.4.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 11/11] [media] lirc_imon: simplify error handling code
2015-06-05 14:27 [PATCH 00/11] Some smatch fixups Mauro Carvalho Chehab
` (10 preceding siblings ...)
2015-06-05 14:27 ` [PATCH 10/11] [media] usbvision: cleanup the code Mauro Carvalho Chehab
@ 2015-06-05 14:27 ` Mauro Carvalho Chehab
11 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-05 14:27 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Jarod Wilson,
Greg Kroah-Hartman, Raphael Poggi, Tapasweni Pathak, Aya Mahfouz,
Amber Thrall, Haneen Mohammed, Gulsah Kose, devel
Instead of using a state machine and a switch with lots of
fall-trough, use gotos and cleanup the error handling loop.
That removes those two smatch warnings:
drivers/staging/media/lirc/lirc_imon.c:933 imon_probe() warn: possible memory leak of 'context'
drivers/staging/media/lirc/lirc_imon.c:933 imon_probe() warn: possible memory leak of 'driver'
And make the error handling code more standard.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/staging/media/lirc/lirc_imon.c b/drivers/staging/media/lirc/lirc_imon.c
index 335b98a54237..62ec9f70dae4 100644
--- a/drivers/staging/media/lirc/lirc_imon.c
+++ b/drivers/staging/media/lirc/lirc_imon.c
@@ -693,10 +693,9 @@ static int imon_probe(struct usb_interface *interface,
int ifnum;
int lirc_minor = 0;
int num_endpts;
- int retval = 0;
+ int retval = -ENOMEM;
int display_ep_found = 0;
int ir_ep_found = 0;
- int alloc_status = 0;
int vfd_proto_6p = 0;
struct imon_context *context = NULL;
int i;
@@ -706,10 +705,8 @@ static int imon_probe(struct usb_interface *interface,
mutex_lock(&driver_lock);
context = kzalloc(sizeof(struct imon_context), GFP_KERNEL);
- if (!context) {
- alloc_status = 1;
- goto alloc_status_switch;
- }
+ if (!context)
+ goto driver_unlock;
/*
* Try to auto-detect the type of display if the user hasn't set
@@ -775,8 +772,7 @@ static int imon_probe(struct usb_interface *interface,
dev_err(dev, "%s: no valid input (IR) endpoint found.\n",
__func__);
retval = -ENODEV;
- alloc_status = 2;
- goto alloc_status_switch;
+ goto free_context;
}
/* Determine if display requires 6 packets */
@@ -790,31 +786,26 @@ static int imon_probe(struct usb_interface *interface,
driver = kzalloc(sizeof(struct lirc_driver), GFP_KERNEL);
if (!driver) {
- alloc_status = 2;
- goto alloc_status_switch;
+ goto free_context;
}
rbuf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
if (!rbuf) {
- alloc_status = 3;
- goto alloc_status_switch;
+ goto free_driver;
}
if (lirc_buffer_init(rbuf, BUF_CHUNK_SIZE, BUF_SIZE)) {
dev_err(dev, "%s: lirc_buffer_init failed\n", __func__);
- alloc_status = 4;
- goto alloc_status_switch;
+ goto free_rbuf;
}
rx_urb = usb_alloc_urb(0, GFP_KERNEL);
if (!rx_urb) {
dev_err(dev, "%s: usb_alloc_urb failed for IR urb\n", __func__);
- alloc_status = 5;
- goto alloc_status_switch;
+ goto free_lirc_buf;
}
tx_urb = usb_alloc_urb(0, GFP_KERNEL);
if (!tx_urb) {
dev_err(dev, "%s: usb_alloc_urb failed for display urb\n",
__func__);
- alloc_status = 6;
- goto alloc_status_switch;
+ goto free_rx_urb;
}
mutex_init(&context->ctx_lock);
@@ -840,11 +831,11 @@ static int imon_probe(struct usb_interface *interface,
lirc_minor = lirc_register_driver(driver);
if (lirc_minor < 0) {
dev_err(dev, "%s: lirc_register_driver failed\n", __func__);
- alloc_status = 7;
- goto unlock;
- } else
- dev_info(dev, "Registered iMON driver (lirc minor: %d)\n",
- lirc_minor);
+ goto free_tx_urb;
+ }
+
+ dev_info(dev, "Registered iMON driver (lirc minor: %d)\n",
+ lirc_minor);
/* Needed while unregistering! */
driver->minor = lirc_minor;
@@ -872,11 +863,9 @@ static int imon_probe(struct usb_interface *interface,
context->rx_endpoint->bInterval);
retval = usb_submit_urb(context->rx_urb, GFP_KERNEL);
-
if (retval) {
dev_err(dev, "usb_submit_urb failed for intf0 (%d)\n", retval);
- alloc_status = 8;
- goto unlock;
+ goto unregister_lirc;
}
usb_set_intfdata(interface, context);
@@ -895,39 +884,31 @@ static int imon_probe(struct usb_interface *interface,
dev_info(dev, "iMON device (%04x:%04x, intf%d) on usb<%d:%d> initialized\n",
vendor, product, ifnum, usbdev->bus->busnum, usbdev->devnum);
-unlock:
- mutex_unlock(&context->ctx_lock);
-alloc_status_switch:
+ /* Everything went fine. Just unlock and return retval (with is 0) */
+ goto driver_unlock;
- switch (alloc_status) {
- case 8:
- lirc_unregister_driver(driver->minor);
- case 7:
- usb_free_urb(tx_urb);
- case 6:
- usb_free_urb(rx_urb);
- /* fall-through */
- case 5:
- if (rbuf)
- lirc_buffer_free(rbuf);
- /* fall-through */
- case 4:
- kfree(rbuf);
- /* fall-through */
- case 3:
- kfree(driver);
- /* fall-through */
- case 2:
- kfree(context);
- context = NULL;
- case 1:
- if (retval != -ENODEV)
- retval = -ENOMEM;
- break;
- case 0:
- retval = 0;
- }
+unregister_lirc:
+ lirc_unregister_driver(driver->minor);
+free_tx_urb:
+ usb_free_urb(tx_urb);
+
+free_rx_urb:
+ usb_free_urb(rx_urb);
+
+free_lirc_buf:
+ lirc_buffer_free(rbuf);
+
+free_rbuf:
+ kfree(rbuf);
+
+free_driver:
+ kfree(driver);
+free_context:
+ kfree(context);
+ context = NULL;
+
+driver_unlock:
mutex_unlock(&driver_lock);
return retval;
--
2.4.2
^ permalink raw reply related [flat|nested] 17+ messages in thread