potential null deref in mpeg_open()
Hello,
I am testing a source checker (http://repo.or.cz/w/smatch.git) and it
found a bug in in mpeg_open() from drivers/media/video/cx23885/cx23885-417.c
dev is null on line 1554, so on line 1558 dprintk() will cause a
kernel oops if it is in debug mode.
drivers/media/video/cx23885/cx23885-417.c
1554 struct cx23885_dev *h, *dev = NULL;
1555 struct list_head *list;
1556 struct cx23885_fh *fh;
1557
1558 dprintk(2, %s()\n, __func__);
Here is how dprintk() is defined earlier.
drivers/media/video/cx23885/cx23885-417.c
59 #define dprintk(level, fmt, arg...)\
60 do { if (v4l_debug = level) \
61 printk(KERN_DEBUG %s: fmt, dev-name , ## arg);\
62 } while (0)
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
double unlock in bttv_poll() and in saa7134-video.c
Hello, My source code checker, smatch (http://repo.or.cz/w/smatch.git), complains about a double unlock in bttv_poll() from drivers/media/video/bt8xx/bttv-driver.c. It unlocks on line 3190 and again on 3201. 3190 mutex_unlock(fh-cap.vb_lock); 3191 buf = (struct bttv_buffer*)fh-cap.read_buf; 3192 } 3193 3194 poll_wait(file, buf-vb.done, wait); 3195 if (buf-vb.state == VIDEOBUF_DONE || 3196 buf-vb.state == VIDEOBUF_ERROR) 3197 rc = POLLIN|POLLRDNORM; 3198 else 3199 rc = 0; 3200 err: 3201 mutex_unlock(fh-cap.vb_lock); I looked at the code but I wasn't sure what the correct way to fix it is. video_poll() from drivers/media/video/saa7134/saa7134-video.c has the same issue. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] tm6000: check an allocation for failure
This allocation had no error checking. It didn't need to be under
the mutex so I moved it out form there. That makes the error handling
easier and is a potential speed up.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/usb/tm6000/tm6000-core.c
b/drivers/media/usb/tm6000/tm6000-core.c
index 22cc011..7c32353 100644
--- a/drivers/media/usb/tm6000/tm6000-core.c
+++ b/drivers/media/usb/tm6000/tm6000-core.c
@@ -40,10 +40,13 @@ int tm6000_read_write_usb(struct tm6000_core *dev, u8
req_type, u8 req,
u8 *data = NULL;
int delay = 5000;
- mutex_lock(dev-usb_lock);
-
- if (len)
+ if (len) {
data = kzalloc(len, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+ }
+
+ mutex_lock(dev-usb_lock);
if (req_type USB_DIR_IN)
pipe = usb_rcvctrlpipe(dev-udev, 0);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: V4L/DVB (8986): cx24116: Adding DVB-S2 demodulator support
Hello Steven Toth, The patch 0d46748c3f87: V4L/DVB (8986): cx24116: Adding DVB-S2 demodulator support from Sep 4, 2008, leads to the following warning: drivers/media/dvb-frontends/cx24116.c:983 cx24116_send_diseqc_msg() error: buffer overflow 'd-msg' 6 = 23 drivers/media/dvb-frontends/cx24116.c 977 /* Validate length */ 978 if (d-msg_len (CX24116_ARGLEN - CX24116_DISEQC_MSGOFS)) 979 return -EINVAL; 980 981 /* DiSEqC message */ 982 for (i = 0; i d-msg_len; i++) 983 state-dsec_cmd.args[CX24116_DISEQC_MSGOFS + i] = d-msg[i]; 984 The state-dsec_cmd.args[] array has 30 elements. The d-msg[] array has only 6 elements. We check that we don't write past the end of the bigger array, but we could read past the end of the smaller array. d-msg_len comes from the user. I don't know if this can result in an information leak? It's weird that we're copying bogus data into the state-dsec_cmd.args[] array. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v2] dvb-usb: check for invalid length in ttusb_process_muxpack()
This patch is driven by a static checker warning.
The ttusb_process_muxpack() function is only called from
ttusb_process_frame(). Before calling, it verifies that len = 2. The
problem is that len == 2 is not valid and would lead to an array
underflow.
Odd number values for len are also invalid and would lead to reading
past the end of the array.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
v2: Moved the check from the caller into the function. Added a check
for odd values. Added an error message. Increment the numinvalid
counter.
diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index 5b682cc..e407185 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -561,6 +561,13 @@ static void ttusb_process_muxpack(struct ttusb *ttusb,
const u8 * muxpack,
{
u16 csum = 0, cc;
int i;
+
+ if (len 4 || len 0x1) {
+ pr_warn(%s: muxpack has invalid len %d\n, __func__, len);
+ numinvalid++;
+ return;
+ }
+
for (i = 0; i len; i += 2)
csum ^= le16_to_cpup((__le16 *) (muxpack + i));
if (csum) {
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [media] tm6000: add support for control events and prio handling
Hello Hans Verkuil,
The patch 770056c47fbb: [media] tm6000: add support for control
events and prio handling from Sep 11, 2012, leads to the following
Smatch warning:
drivers/media/usb/tm6000/tm6000-video.c:1462 __tm6000_poll()
error: potentially dereferencing uninitialized 'buf'.
drivers/media/usb/tm6000/tm6000-video.c
1453 if (!is_res_read(fh-dev, fh)) {
1454 /* streaming capture */
1455 if (list_empty(fh-vb_vidq.stream))
1456 return res | POLLERR;
1457 buf = list_entry(fh-vb_vidq.stream.next, struct
tm6000_buffer, vb.stream);
1458 } else if (req_events (POLLIN | POLLRDNORM)) {
1459 /* read() capture */
1460 return res | videobuf_poll_stream(file, fh-vb_vidq,
wait);
1461 }
If we don't hit either side of the if else statement then buf is
uninitialized.
1462 poll_wait(file, buf-vb.done, wait);
1463 if (buf-vb.state == VIDEOBUF_DONE ||
1464 buf-vb.state == VIDEOBUF_ERROR)
1465 return res | POLLIN | POLLRDNORM;
1466 return res;
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] mceusb: move check earlier to make smatch happy
Smatch complains that cmdbuf[cmdcount - length] might go past the end
of the array. It's an easy warning to silence by moving the limit
check earlier.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index bdd1ed8..5b5b6e6 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -828,16 +828,16 @@ static int mceusb_tx_ir(struct rc_dev *dev, unsigned
*txbuf, unsigned count)
(txbuf[i] -= MCE_MAX_PULSE_LENGTH));
}
- /* Fix packet length in last header */
- length = cmdcount % MCE_CODE_LENGTH;
- cmdbuf[cmdcount - length] -= MCE_CODE_LENGTH - length;
-
/* Check if we have room for the empty packet at the end */
if (cmdcount = MCE_CMDBUF_SIZE) {
ret = -EINVAL;
goto out;
}
+ /* Fix packet length in last header */
+ length = cmdcount % MCE_CODE_LENGTH;
+ cmdbuf[cmdcount - length] -= MCE_CODE_LENGTH - length;
+
/* All mce commands end with an empty packet (0x80) */
cmdbuf[cmdcount++] = MCE_IRDATA_TRAILER;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] MAINTAINERS: Remove Jarod Wilson and orphan LIRC drivers
On Tue, Feb 12, 2013 at 01:20:36PM -0800, Joe Perches wrote: His email bounces and he hasn't done work on these sections in a couple of years. I've added him to the CC list. Can we just update MAINTAINERS with the correct email address? It's been useful to CC him on stuff. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -next] [media] go7007: fix invalid use of sizeof in go7007_usb_i2c_master_xfer()
On Tue, Mar 26, 2013 at 02:42:47PM +0800, Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn sizeof() when applied to a pointer typed expression gives the size of the pointer, not that of the pointed data. This fix isn't right. buf is a char pointer. I don't know what this code is doing. Instead of sizeof(*buf) it should be something like buflen, msg[i].len, msg[i].len + 1 or msg[i].len + 3. I'm not sure which is correct here or what it's doing, sorry. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -next] [media] go7007: fix invalid use of sizeof in go7007_usb_i2c_master_xfer()
On Tue, Mar 26, 2013 at 10:04:15AM +0300, Dan Carpenter wrote: On Tue, Mar 26, 2013 at 02:42:47PM +0800, Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn sizeof() when applied to a pointer typed expression gives the size of the pointer, not that of the pointed data. This fix isn't right. buf is a char pointer. I don't know what this code is doing. Instead of sizeof(*buf) it should be something like buflen, msg[i].len, msg[i].len + 1 or msg[i].len + 3. It should be msg[i].len + 1, I think. On the line before it writes buflen bytes to the hardware. Then it clears the transfer buffer and reads msg[i].len + 1 bytes from the hardware. Then it saves the memory, except for the first byte, in msg[i].buf. So it should clear msg[i].len + 1 bytes so that the old data isn't confused with the data that we read from the hardware. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v2] [media] rc-core: prevent divide by zero bug in s_tx_carrier()
Several of the drivers use carrier as a divisor in their s_tx_carrier() functions. We should do a sanity check here like we do for LIRC_SET_REC_CARRIER. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- v2: Ben Hutchings pointed out that my first patch was not a complete fix. diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c index 6ad4a07..28dc0f0 100644 --- a/drivers/media/rc/ir-lirc-codec.c +++ b/drivers/media/rc/ir-lirc-codec.c @@ -211,6 +211,9 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, if (!dev-s_tx_carrier) return -EINVAL; + if (val = 0) + return -EINVAL; + return dev-s_tx_carrier(dev, val); case LIRC_SET_SEND_DUTY_CYCLE: -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] staging: media: go7007: Some additional code for TW2804 driver functionality
;
+ break;
+ case V4L2_CID_COLOR_KILLER:
+ reg = read_reg(client, addr, state-channel);
+ if (reg 0)
+ reg = (reg ~(0x03)) | (ctrl-val == 0 ? 0x02 : 0x03);
+ else
+ return reg;
+ break;
+ default:
+ break;
+ }
+
+ reg = reg 255 ? 255 : (reg 0 ? 0 : reg);
+ reg = write_reg(client, addr, (u8)reg,
Not needed.
+ ctrl-id == V4L2_CID_GAIN ||
+ ctrl-id == V4L2_CID_CHROMA_GAIN ||
+ ctrl-id == V4L2_CID_RED_BALANCE ||
+ ctrl-id == V4L2_CID_BLUE_BALANCE ? 0 : state-channel);
+
+ if (reg 0) {
+ v4l2_err(sd, Can`t set_ctrl value:id=%d;value=%d\n, ctrl-id,
+ ctrl-val);
+ return reg;
+ }
+ return 0;
+}
+
+static int tw2804_s_std(struct v4l2_subdev *sd, v4l2_std_id norm)
+{
+ struct wis_tw2804 *dec = to_state(sd);
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+
No blank line here.
+ u8 regs[] = {
+ 0x01, normV4L2_STD_NTSC ? 0xc4 : 0x84,
+ 0x09, normV4L2_STD_NTSC ? 0x07 : 0x04,
+ 0x0a, normV4L2_STD_NTSC ? 0xf0 : 0x20,
+ 0x0b, normV4L2_STD_NTSC ? 0x07 : 0x04,
+ 0x0c, normV4L2_STD_NTSC ? 0xf0 : 0x20,
+ 0x0d, normV4L2_STD_NTSC ? 0x40 : 0x4a,
+ 0x16, normV4L2_STD_NTSC ? 0x00 : 0x40,
+ 0x17, normV4L2_STD_NTSC ? 0x00 : 0x40,
+ 0x20, normV4L2_STD_NTSC ? 0x07 : 0x0f,
+ 0x21, normV4L2_STD_NTSC ? 0x07 : 0x0f,
This is hard to read without proper white space:
0x01, norm V4L2_STD_NTSC ? 0xc4 : 0x84,
0x09, norm V4L2_STD_NTSC ? 0x07 : 0x04,
+ 0xff, 0xff,
+ };
Put a blank line here.
+ write_regs(client, regs, dec-channel);
+ dec-norm = norm;
+ return 0;
+}
+
+static int tw2804_g_chip_ident(struct v4l2_subdev *sd,
+ struct v4l2_dbg_chip_ident *chip)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ return v4l2_chip_ident_i2c_client(client, chip,
+ V4L2_IDENT_TW2804, 0x0e);
+}
+
+static const struct v4l2_ctrl_ops tw2804_ctrl_ops = {
+ .g_volatile_ctrl = tw2804_g_volatile_ctrl,
+ .s_ctrl = tw2804_s_ctrl,
+};
+
+static const struct v4l2_subdev_core_ops tw2804_core_ops = {
+ .log_status = tw2804_log_status,
+ .g_chip_ident = tw2804_g_chip_ident,
+ .g_ext_ctrls = v4l2_subdev_g_ext_ctrls,
+ .try_ext_ctrls = v4l2_subdev_try_ext_ctrls,
+ .s_ext_ctrls = v4l2_subdev_s_ext_ctrls,
+ .g_ctrl = v4l2_subdev_g_ctrl,
+ .s_ctrl = v4l2_subdev_s_ctrl,
+ .queryctrl = v4l2_subdev_queryctrl,
+ .querymenu = v4l2_subdev_querymenu,
+ .s_std = tw2804_s_std,
+};
+
+static int tw2804_s_video_routing(struct v4l2_subdev *sd, u32 input, u32
output,
+ u32 config)
+{
+ struct wis_tw2804 *dec = to_state(sd);
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ s32 reg = 0;
The initialization is not needed here.
+
+ if (0 input || input 1)
+ return -EINVAL;
+
+ if (input == dec-input)
+ return 0;
+
+ reg = read_reg(client, 0x22, dec-channel);
Move the error handling next to the function call. It makes the
code simpler and you can pull everything in an indent level.
reg = read_reg(client, 0x22, dec-channel);
if (reg 0)
return reg;
if (input == 0)
reg = ~(1 2);
else
reg |= 1 2;
reg = write_reg(client, 0x22, reg, dec-channel);
if (reg 0)
return reg;
dec-input = input;
return 0;
regards,
dan carpenter
+
+ if (reg = 0) {
+ if (input == 0)
+ reg = ~(12);
+ else
+ reg |= 12;
+ reg = write_reg(client, 0x22, (u8)reg, dec-channel);
+ }
+
+ if (reg = 0)
+ dec-input = input;
+ else
+ return reg;
return 0;
}
+static int tw2804_s_mbus_fmt(struct v4l2_subdev *sd,
+ struct v4l2_mbus_framefmt *fmt)
+{
+ /*TODO need select between 3fmt:
+ * bt_656,
+ * bt_601_8bit,
+ * bt_656_dual,
+ */
+ return 0;
+}
+
+static int tw2804_s_stream(struct v4l2_subdev *sd, int enable)
+{
+ struct wis_tw2804 *dec = to_state(sd);
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ u32 reg = read_reg(client, 0x78, 0);
+
+ if (enable == 1)
+ write_reg(client, 0x78, reg ~(1dec-channel), 0);
+ else
+ write_reg(client, 0x78, reg | (1dec-channel), 0);
+
+ return 0;
+}
+
+static const struct
[patch v3] [media] rc: divide by zero bugs in s_tx_carrier()
carrier comes from a get_user() in ir_lirc_ioctl(). We need to test
that it's not zero before using it as a divisor. It might have been
nice to test for this ir_lirc_ioctl() but the mceusb driver uses zero to
disable carrier modulation.
The bug in redrat3 is a little more subtle. The -carrier is passed to
mod_freq_to_val() which uses it as a divisor.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
v2: tried to add the check to ir_lirc_ioctl() but that doesn't work.
v3: the same as v1 except that I've added a fix for redrat3 as well.
diff --git a/drivers/media/rc/ene_ir.c b/drivers/media/rc/ene_ir.c
index 647dd95..d05ac15 100644
--- a/drivers/media/rc/ene_ir.c
+++ b/drivers/media/rc/ene_ir.c
@@ -881,10 +881,13 @@ static int ene_set_tx_mask(struct rc_dev *rdev, u32
tx_mask)
static int ene_set_tx_carrier(struct rc_dev *rdev, u32 carrier)
{
struct ene_device *dev = rdev-priv;
- u32 period = 200 / carrier;
+ u32 period;
dbg(TX: attempt to set tx carrier to %d kHz, carrier);
+ if (carrier == 0)
+ return -EINVAL;
+ period = 200 / carrier;
if (period (period ENE_CIRMOD_PRD_MAX ||
period ENE_CIRMOD_PRD_MIN)) {
diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
index 699eef3..2ea913a 100644
--- a/drivers/media/rc/nuvoton-cir.c
+++ b/drivers/media/rc/nuvoton-cir.c
@@ -517,6 +517,9 @@ static int nvt_set_tx_carrier(struct rc_dev *dev, u32
carrier)
struct nvt_dev *nvt = dev-priv;
u16 val;
+ if (carrier == 0)
+ return -EINVAL;
+
nvt_cir_reg_write(nvt, 1, CIR_CP);
val = 300 / (carrier) - 1;
nvt_cir_reg_write(nvt, val 0xff, CIR_CC);
diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index 2878b0e..bf8bc74 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -890,6 +890,9 @@ static int redrat3_set_tx_carrier(struct rc_dev *rcdev, u32
carrier)
struct device *dev = rr3-dev;
rr3_dbg(dev, Setting modulation frequency to %u, carrier);
+ if (carrier == 0)
+ return -EINVAL;
+
rr3-carrier = carrier;
return carrier;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] rc-core: fix return codes in ir_lirc_ioctl()
These should be -ENOSYS because not -EINVAL. Reported-by: Sean Young s...@mess.org Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c index 6ad4a07..c0dc1b9 100644 --- a/drivers/media/rc/ir-lirc-codec.c +++ b/drivers/media/rc/ir-lirc-codec.c @@ -203,13 +203,13 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, /* TX settings */ case LIRC_SET_TRANSMITTER_MASK: if (!dev-s_tx_mask) - return -EINVAL; + return -ENOSYS; return dev-s_tx_mask(dev, val); case LIRC_SET_SEND_CARRIER: if (!dev-s_tx_carrier) - return -EINVAL; + return -ENOSYS; return dev-s_tx_carrier(dev, val); -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/media: Removes useless kfree()
On Tue, Sep 11, 2012 at 08:00:32PM +0200, Peter Senna Tschudin wrote: diff --git a/drivers/media/dvb-frontends/lg2160.c b/drivers/media/dvb-frontends/lg2160.c index cc11260..748da5d 100644 --- a/drivers/media/dvb-frontends/lg2160.c +++ b/drivers/media/dvb-frontends/lg2160.c @@ -1451,7 +1451,6 @@ struct dvb_frontend *lg2160_attach(const struct lg2160_config *config, return state-frontend; fail: lg_warn(unable to detect LG216x hardware\n); - kfree(state); return NULL; } I wish you had fixed this the same as the others and removed the goto. Also the printk is redundant and wrong. Remove it too. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] drivers/media/platform/davinci/vpbe.c: Removes useless kfree()
On Wed, Sep 12, 2012 at 05:50:54PM +0200, Peter Senna Tschudin wrote:
Marcos,
Now that you removed this kfree, you could remove this label too. Very
nice your cleanup :)
Thanks!
vpbe_fail_sd_register:
kfree(vpbe_dev-encoders);
vpbe_fail_v4l2_device:
The problem removing the label is that it will require some more work
naming the labels. See:
if (!vpbe_dev-amp) {
...
goto vpbe_fail_amp_register;
If I just remove the label vpbe_fail_amp_register, the label names
will not make sense any more as the next label is
vpbe_fail_sd_register. So I will need to change the name to something
different or rename all labels to out1, out2, out3 or err1, err2,
err3, or
Any suggestions?
Labal names should not be numbers because this is not GW-BASIC. The
label should reflect what happens on the next line. Labeling the
place after the goto location where you started from is always
nonsense.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Freeze or Oops on recent kernels
I'm not going to file a bug for this on bugzilla because it's ancient and not a new bug. But I can forward it to linux-media and the get_maintainer.pl people for cx23885_video. HINT: We only care about the very most recent kernel. If you can take a photo of the stack trace, then file a bug report and attach the .jpg. regards, dan carpenter On Fri, Sep 07, 2012 at 09:24:13PM +1000, yvahk-xre...@zacglen.net wrote: I am getting either a a kernel Oops or freeze (without any console output) on recent kernels. I have tested on 2.6.32.26 PAE, 3.1.9 PAE, and 3.4.9 PAE all with similar results. The hardware involved comprises mainly: 12 GB ram Intel DX58S02 motherboard Intel Xeon E5220 2 x onboard ethernet Intel PRO/1000 PT Dual Port ethernet Hauppauge HVR1700 DVB-T PCIe card Technisat SkyStar2 DVB-T PCI card The oops or freeze occurs when both DVB cards are recording simultaneously. With either card installed on their own there is never any problem. I should also add that the exact same kernel and cards on a Gigabyte GA-P31-S3G motherboard + Intel Pentium 4 the problem NEVER occurs. So there may be a DX58S02/timing/interrupt issue. When there is an oops it is like this (hand-transcribed from 2.6.32.26 PAE kernel): c0789444 panic + 3e/e9 oops_end + 97/a6 no_context + 13b/145 __bad_area_nosemaphore + ec/f4 ? do_page_fault + 0/29f bad_area_nosemaphore + 12/15 do_page_fault + 139/29f ? do_page_fault + 139/29f c078b54b error_code + 73/78 f82084fb ? cx23885_video_irq + d8/1dc f820b04d cx23885_irq + 3df/3fe c04834ac handle_irq_event + 57/fd handle_fasteoi_irq + 6f/a2 handle_irq + 40/4d do_IRQ + 46/9a common_interrupt + 30/38 c045b7a9 ? prepare_to_wait + 14c f7fef5a5 ? videobug_waitm + 90/133 c045b601 ? autoremove_waker_function + 0/34 f805e5bc videobuf_dvb_thread + 73/135 f805e4f9 ? videobuf_dvb_thread + 0/135 c045b3c9 kthread + 64/69 ? kthread + 0/69 kernel_thread_helper + 7/10 Although I am a very experienced programmer I have next to zero kernel expertise except for minor patching of a few drivers. My guess from the stack trace is that there might be an issue with page fault recursion (if that is at all possible). Anyhow I don't want to waste too much of my time or anybody elses on this - although with the problem occuring with 3.4.9 kernel which has significant interrupt handling changes it probably is something that somebody might want to know about. If anybody can spot a clue as to where I should be looking and how I should go about isolating the problem (if only kernel core dumped!) please let me know and I will possibly try and assist. I need some guidance. Regards John W. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [media] v4l2-subdev: add support for the new edid ioctls
Hi Hans,
The patch ed45ce2cc0b3: [media] v4l2-subdev: add support for the new
edid ioctls from Aug 10, 2012, needs an overflow check the same as the
other cases in that switch statement.
drivers/media/v4l2-core/v4l2-ioctl.c
2200 case VIDIOC_SUBDEV_G_EDID:
2201 case VIDIOC_SUBDEV_S_EDID: {
2202 struct v4l2_subdev_edid *edid = parg;
2203
2204 if (edid-blocks) {
2205 *user_ptr = (void __user *)edid-edid;
2206 *kernel_ptr = (void *)edid-edid;
2207 *array_size = edid-blocks * 128;
^^
This can overflow.
2208 ret = 1;
2209 }
2210 break;
2211 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] cx25821: testing the wrong variable
-input_filename could be NULL here. The intent was to test
-_filename.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
I'm not totally convinced that using /root/vid411.yuv is the right idea.
diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream.c
b/drivers/media/pci/cx25821/cx25821-video-upstream.c
index 52c13e0..6759fff 100644
--- a/drivers/media/pci/cx25821/cx25821-video-upstream.c
+++ b/drivers/media/pci/cx25821/cx25821-video-upstream.c
@@ -808,7 +808,7 @@ int cx25821_vidupstream_init_ch1(struct cx25821_dev *dev,
int channel_select,
}
/* Default if filename is empty string */
- if (strcmp(dev-input_filename, ) == 0) {
+ if (strcmp(dev-_filename, ) == 0) {
if (dev-_isNTSC) {
dev-_filename =
(dev-_pixel_format == PIXEL_FRMT_411) ?
diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
index c8c94fb..d33fc1a 100644
--- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
+++ b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
@@ -761,7 +761,7 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev,
int channel_select,
}
/* Default if filename is empty string */
- if (strcmp(dev-input_filename_ch2, ) == 0) {
+ if (strcmp(dev-_filename_ch2, ) == 0) {
if (dev-_isNTSC_ch2) {
dev-_filename_ch2 = (dev-_pixel_format_ch2 ==
PIXEL_FRMT_411) ? /root/vid411.yuv :
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [media] cx25821: testing the wrong variable
On Sat, Sep 29, 2012 at 12:52:38PM +0200, walter harms wrote:
Am 29.09.2012 09:12, schrieb Dan Carpenter:
-input_filename could be NULL here. The intent was to test
-_filename.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
I'm not totally convinced that using /root/vid411.yuv is the right idea.
diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream.c
b/drivers/media/pci/cx25821/cx25821-video-upstream.c
index 52c13e0..6759fff 100644
--- a/drivers/media/pci/cx25821/cx25821-video-upstream.c
+++ b/drivers/media/pci/cx25821/cx25821-video-upstream.c
@@ -808,7 +808,7 @@ int cx25821_vidupstream_init_ch1(struct cx25821_dev
*dev, int channel_select,
}
/* Default if filename is empty string */
- if (strcmp(dev-input_filename, ) == 0) {
+ if (strcmp(dev-_filename, ) == 0) {
if (dev-_isNTSC) {
dev-_filename =
(dev-_pixel_format == PIXEL_FRMT_411) ?
diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
index c8c94fb..d33fc1a 100644
--- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
+++ b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
@@ -761,7 +761,7 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev
*dev, int channel_select,
}
/* Default if filename is empty string */
- if (strcmp(dev-input_filename_ch2, ) == 0) {
+ if (strcmp(dev-_filename_ch2, ) == 0) {
if (dev-_isNTSC_ch2) {
dev-_filename_ch2 = (dev-_pixel_format_ch2 ==
PIXEL_FRMT_411) ? /root/vid411.yuv :
In this case stcmp seems a bit of a overkill. A simple
*(dev-_filename_ch2) == 0
should be ok ?
I prefer strcmp() actually. More readable.
regards,
dan carpenter
re,
wh
--
To unsubscribe from this list: send the line unsubscribe kernel-janitors in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [media] af9035: add Avermedia Volar HD (A867R) support
Hello Hans-Frieder Vogt, The patch 540fd4ba0533: [media] af9035: add Avermedia Volar HD (A867R) support from Apr 2, 2012, leads to the following Clang warning: drivers/media/dvb-frontends/af9033.c:467:20: warning: comparison of unsigned expression = 0 is always true [-Wtautological-compare] drivers/media/dvb-frontends/af9033.c 464 while (if_frequency (adc_freq / 2)) ^ if_frequency is unsigned. I worry that this loop doesn't handle integer underflow properly. 465 if_frequency -= adc_freq; 466 467 if (if_frequency = 0) ^ This is always true. 468 spec_inv *= -1; 469 else 470 if_frequency *= -1; regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: V4L/DVB (13678): Add support for yet another DvbWorld, TeVii and Prof USB devices
Hello Igor M. Liplianin,
The patch 141cc35e2d29: V4L/DVB (13678): Add support for yet another
DvbWorld, TeVii and Prof USB devices from Nov 27, 2009, leads to the
following Sparse warning:
drivers/media/usb/dvb-usb/dw2102.c:288:36: error: bad constant
expression
CHECK drivers/media/usb/dvb-usb/dw2102.c
drivers/media/usb/dvb-usb/dw2102.c:288:36: error: bad constant expression
drivers/media/usb/dvb-usb/dw2102.c:305:44: error: bad constant expression
drivers/media/usb/dvb-usb/dw2102.c:315:44: error: bad constant expression
drivers/media/usb/dvb-usb/dw2102.c:381:53: error: bad constant expression
drivers/media/usb/dvb-usb/dw2102.c:410:52: error: bad constant expression
drivers/media/usb/dvb-usb/dw2102.c:443:36: error: bad constant expression
drivers/media/usb/dvb-usb/dw2102.c:461:44: error: bad constant expression
drivers/media/usb/dvb-usb/dw2102.c:543:47: error: bad constant expression
drivers/media/usb/dvb-usb/dw2102.c:570:52: error: bad constant expression
drivers/media/usb/dvb-usb/dw2102.c:582:52: error: bad constant expression
CC [M] drivers/media/usb/dvb-usb/dw2102.o
284 switch (num) {
285 case 2: {
286 /* read */
287 /* first write first register number */
288 u8 ibuf[msg[1].len + 2], obuf[3];
^^
The kernel has an 8k stack so the worry is that len could larger than
that.
289 obuf[0] = msg[0].addr 1;
290 obuf[1] = msg[0].len;
291 obuf[2] = msg[0].buf[0];
292 dw210x_op_rw(d-udev, 0xc2, 0, 0,
293 obuf, msg[0].len + 2, DW210X_WRITE_MSG);
294 /* second read registers */
295 dw210x_op_rw(d-udev, 0xc3, 0xd1 , 0,
296 ibuf, msg[1].len + 2, DW210X_READ_MSG);
297 memcpy(msg[1].buf, ibuf + 2, msg[1].len);
298
299 break;
300 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] staging: lirc: clean error handling in probe()
I have reorganized the error handling into a simpler and more canonical
format.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/staging/media/lirc/lirc_igorplugusb.c
b/drivers/staging/media/lirc/lirc_igorplugusb.c
index 2faa391..4cf3933 100644
--- a/drivers/staging/media/lirc/lirc_igorplugusb.c
+++ b/drivers/staging/media/lirc/lirc_igorplugusb.c
@@ -390,7 +390,6 @@ static int igorplugusb_remote_probe(struct usb_interface
*intf,
int devnum, pipe, maxp;
int minor = 0;
char buf[63], name[128] = ;
- int mem_failure = 0;
int ret;
dprintk(DRIVER_NAME : usb probe called.\n);
@@ -416,24 +415,17 @@ static int igorplugusb_remote_probe(struct usb_interface
*intf,
dprintk(DRIVER_NAME [%d]: bytes_in_key=%zu maxp=%d\n,
devnum, CODE_LENGTH, maxp);
- mem_failure = 0;
ir = kzalloc(sizeof(struct igorplug), GFP_KERNEL);
- if (!ir) {
- mem_failure = 1;
- goto mem_failure_switch;
- }
+ if (!ir)
+ return -ENOMEM;
driver = kzalloc(sizeof(struct lirc_driver), GFP_KERNEL);
- if (!driver) {
- mem_failure = 2;
- goto mem_failure_switch;
- }
+ if (!driver)
+ goto err_free_ir;
ir-buf_in = usb_alloc_coherent(dev, DEVICE_BUFLEN + DEVICE_HEADERLEN,
GFP_ATOMIC, ir-dma_in);
- if (!ir-buf_in) {
- mem_failure = 3;
- goto mem_failure_switch;
- }
+ if (!ir-buf_in)
+ goto err_free_driver;
strcpy(driver-name, DRIVER_NAME );
driver-minor = -1;
@@ -451,23 +443,7 @@ static int igorplugusb_remote_probe(struct usb_interface
*intf,
minor = lirc_register_driver(driver);
if (minor 0)
- mem_failure = 9;
-
-mem_failure_switch:
-
- switch (mem_failure) {
- case 9:
- usb_free_coherent(dev, DEVICE_BUFLEN + DEVICE_HEADERLEN,
- ir-buf_in, ir-dma_in);
- case 3:
- kfree(driver);
- case 2:
- kfree(ir);
- case 1:
- printk(DRIVER_NAME [%d]: out of memory (code=%d)\n,
- devnum, mem_failure);
- return -ENOMEM;
- }
+ goto err_free_usb;
driver-minor = minor;
ir-d = driver;
@@ -500,6 +476,16 @@ mem_failure_switch:
usb_set_intfdata(intf, ir);
return 0;
+
+err_free_usb:
+ usb_free_coherent(dev, DEVICE_BUFLEN + DEVICE_HEADERLEN, ir-buf_in,
+ ir-dma_in);
+err_free_driver:
+ kfree(driver);
+err_free_ir:
+ kfree(ir);
+
+ return -ENOMEM;
}
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [media] staging: lirc: clean error handling in probe()
On Wed, Jun 26, 2013 at 11:00:40AM +0300, Andy Shevchenko wrote: On Wed, 2013-06-26 at 10:53 +0300, Dan Carpenter wrote: I have reorganized the error handling into a simpler and more canonical format. Since you reorganize error handling, might be worth to convert it to devm_*? If you want I could do the patch. Yeah. Using devm_kzalloc() would make it a lot simpler. That would be great if you could re-write it that way. Thanks! Please drop my patch in that case. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] staging: lirc: clean error handling in probe()
On Wed, Jun 26, 2013 at 05:37:36PM +0300, Andy Shevchenko wrote: From: Dan Carpenter dan.carpen...@oracle.com We have reorganized the error handling into a simpler and more canonical format. Additionally we removed extra empty lines, switched to devm_kzalloc(), and substitute 'minor' by 'ret' in the igorplugusb_remote_probe() function. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com I don't want credit for this, because I didn't do the work. Signed-off-by is a legal thing like signing a document... But I have reviewed it and it looks good. Acked-by: Dan Carpenter dan.carpen...@oracle.com regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] staging: lirc: clean error handling in probe()
On Wed, Jun 26, 2013 at 06:29:12PM +0300, Andy Shevchenko wrote: On Wed, 2013-06-26 at 18:10 +0300, Dan Carpenter wrote: On Wed, Jun 26, 2013 at 05:37:36PM +0300, Andy Shevchenko wrote: From: Dan Carpenter dan.carpen...@oracle.com We have reorganized the error handling into a simpler and more canonical format. Additionally we removed extra empty lines, switched to devm_kzalloc(), and substitute 'minor' by 'ret' in the igorplugusb_remote_probe() function. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com I don't want credit for this, because I didn't do the work. Signed-off-by is a legal thing like signing a document... I took your patch and modified it, so, you have done some work too. But I could resend a version with my authorship. I don't think any of my work survived the re-write, but I'm happy enough to take credit for other people's work. :) Don't bother resending. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] bt8xx: info leak in ca_get_slot_info()
p_ca_slot_info was allocated with kmalloc() so we need to clear it
before passing it to the user.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/pci/bt8xx/dst_ca.c b/drivers/media/pci/bt8xx/dst_ca.c
index 0e788fc..6b9dc3f 100644
--- a/drivers/media/pci/bt8xx/dst_ca.c
+++ b/drivers/media/pci/bt8xx/dst_ca.c
@@ -302,8 +302,11 @@ static int ca_get_slot_info(struct dst_state *state,
struct ca_slot_info *p_ca_s
p_ca_slot_info-flags = CA_CI_MODULE_READY;
p_ca_slot_info-num = 1;
p_ca_slot_info-type = CA_CI;
- } else
+ } else {
p_ca_slot_info-flags = 0;
+ p_ca_slot_info-num = 0;
+ p_ca_slot_info-type = 0;
+ }
if (copy_to_user(arg, p_ca_slot_info, sizeof (struct ca_slot_info)))
return -EFAULT;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [media] bt8xx: info leak in ca_get_slot_info()
On Thu, Jul 25, 2013 at 07:29:09PM +0200, walter harms wrote:
Am 25.07.2013 18:46, schrieb Dan Carpenter:
p_ca_slot_info was allocated with kmalloc() so we need to clear it
before passing it to the user.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/pci/bt8xx/dst_ca.c
b/drivers/media/pci/bt8xx/dst_ca.c
index 0e788fc..6b9dc3f 100644
--- a/drivers/media/pci/bt8xx/dst_ca.c
+++ b/drivers/media/pci/bt8xx/dst_ca.c
@@ -302,8 +302,11 @@ static int ca_get_slot_info(struct dst_state *state,
struct ca_slot_info *p_ca_s
p_ca_slot_info-flags = CA_CI_MODULE_READY;
p_ca_slot_info-num = 1;
p_ca_slot_info-type = CA_CI;
- } else
+ } else {
p_ca_slot_info-flags = 0;
+ p_ca_slot_info-num = 0;
+ p_ca_slot_info-type = 0;
+ }
if (copy_to_user(arg, p_ca_slot_info, sizeof (struct ca_slot_info)))
return -EFAULT;
note: i have no clue how p_ca_slot_info looks like,
but to avoid information leaks via compiler padding etc. i could be more wise
to do a memset(p_ca_slot_info,0,sizeof (struct ca_slot_info))
and then set the
There is no compiler padding. My static checker looks for that.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] trivial: adjust code alignment
On Mon, Aug 05, 2013 at 06:24:43PM +0200, walter harms wrote: Hello Julia, IMHO keep the patch as it is. It does not change any code that is good. Suspicious code that comes up here can be addressed in a separate patch. Gar... No, if we silence static checker warnings without fixing the bug then we are hiding real problems and making them more difficult to find. Just drop this chunk. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] snd_tea575x: precedence bug in fmr2_tea575x_get_pins()
The | operation has higher precedence that ?: so this couldn't return both flags set at once as intended. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- Static checker stuff. Untested. diff --git a/drivers/media/radio/radio-sf16fmr2.c b/drivers/media/radio/radio-sf16fmr2.c index 9c09904..72af59d 100644 --- a/drivers/media/radio/radio-sf16fmr2.c +++ b/drivers/media/radio/radio-sf16fmr2.c @@ -74,8 +74,8 @@ static u8 fmr2_tea575x_get_pins(struct snd_tea575x *tea) struct fmr2 *fmr2 = tea-private_data; u8 bits = inb(fmr2-io); - return (bits STR_DATA) ? TEA575X_DATA : 0 | - (bits STR_MOST) ? TEA575X_MOST : 0; + return ((bits STR_DATA) ? TEA575X_DATA : 0) | + ((bits STR_MOST) ? TEA575X_MOST : 0); } static void fmr2_tea575x_set_direction(struct snd_tea575x *tea, bool output) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] s5k6aa: off by one in s5k6aa_enum_frame_interval()
The check is off by one so we could read one space past the end of the array. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/media/i2c/s5k6aa.c b/drivers/media/i2c/s5k6aa.c index 789c02a..629a5cd 100644 --- a/drivers/media/i2c/s5k6aa.c +++ b/drivers/media/i2c/s5k6aa.c @@ -1003,7 +1003,7 @@ static int s5k6aa_enum_frame_interval(struct v4l2_subdev *sd, const struct s5k6aa_interval *fi; int ret = 0; - if (fie-index ARRAY_SIZE(s5k6aa_intervals)) + if (fie-index = ARRAY_SIZE(s5k6aa_intervals)) return -EINVAL; v4l_bound_align_image(fie-width, S5K6AA_WIN_WIDTH_MIN, -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] i2c/ov9650: off by one in ov965x_enum_frame_sizes()
The should be = otherwise we read one space beyond the end of the
array.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 1dbb811..4da90c6 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -1083,7 +1083,7 @@ static int ov965x_enum_frame_sizes(struct v4l2_subdev *sd,
{
int i = ARRAY_SIZE(ov965x_formats);
- if (fse-index ARRAY_SIZE(ov965x_framesizes))
+ if (fse-index = ARRAY_SIZE(ov965x_framesizes))
return -EINVAL;
while (--i)
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [media] s5p-csis: Add support for non-image data packets capture
Hello Sylwester Nawrocki,
I had a question about 36fa80927638: [media] s5p-csis: Add support for
non-image data packets capture from Sep 21, 2012.
S5PCSIS_INTSRC_NON_IMAGE_DATA is defined in mipi-csis.c:
#define S5PCSIS_INTSRC_NON_IMAGE_DATA (0xff 28)
And it's only used in one place.
drivers/media/platform/exynos4-is/mipi-csis.c
692 u32 status;
693
694 status = s5pcsis_read(state, S5PCSIS_INTSRC);
695 spin_lock_irqsave(state-slock, flags);
696
697 if ((status S5PCSIS_INTSRC_NON_IMAGE_DATA) pktbuf-data) {
^
status is a u32 so 0xff000 is 4 bits too much. In other words,
the mask is effectively (0xf 28). Was that intended or should it
be (0xff 24)?
698 u32 offset;
699
700 if (status S5PCSIS_INTSRC_EVEN)
701 offset = S5PCSIS_PKTDATA_EVEN;
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] exynos4-is: print error message on timeout
There is a stray '!' character so the error message never gets printed. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- Static checker stuff. Not tested. diff --git a/drivers/media/platform/exynos4-is/fimc-is-regs.c b/drivers/media/platform/exynos4-is/fimc-is-regs.c index 63c68ec..42f2925 100644 --- a/drivers/media/platform/exynos4-is/fimc-is-regs.c +++ b/drivers/media/platform/exynos4-is/fimc-is-regs.c @@ -236,7 +236,7 @@ int fimc_is_itf_mode_change(struct fimc_is *is) fimc_is_hw_change_mode(is); ret = fimc_is_wait_event(is, IS_ST_CHANGE_MODE, 1, FIMC_IS_CONFIG_TIMEOUT); - if (!ret 0) + if (ret 0) dev_err(is-pdev-dev, %s(): mode change (%d) timeout\n, __func__, is-config_index); return ret; -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] s3c-camif: forever loop in camif_hw_set_source_format()
Because i is unsigned then i-- = 0 is always true. If we don't
find what we are looking for then we loop forever.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Why do we count backwards anyway? Counting upwards is easier.
diff --git a/drivers/media/platform/s3c-camif/camif-regs.c
b/drivers/media/platform/s3c-camif/camif-regs.c
index a9e3b16..ebf5b18 100644
--- a/drivers/media/platform/s3c-camif/camif-regs.c
+++ b/drivers/media/platform/s3c-camif/camif-regs.c
@@ -106,15 +106,15 @@ static const u32 src_pixfmt_map[8][2] = {
void camif_hw_set_source_format(struct camif_dev *camif)
{
struct v4l2_mbus_framefmt *mf = camif-mbus_fmt;
- unsigned int i = ARRAY_SIZE(src_pixfmt_map);
+ int i;
u32 cfg;
- while (i-- = 0) {
+ for (i = ARRAY_SIZE(src_pixfmt_map) - 1; i = 0; i--) {
if (src_pixfmt_map[i][0] == mf-code)
break;
}
-
- if (i == 0 src_pixfmt_map[i][0] != mf-code) {
+ if (i 0) {
+ i = 0;
dev_err(camif-dev,
Unsupported pixel code, falling back to %#08x\n,
src_pixfmt_map[i][0]);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] mx3-camera: locking typo in mx3_videobuf_queue()
There is a return in the middle where we haven't restored the IRQs to their original state. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/media/platform/soc_camera/mx3_camera.c b/drivers/media/platform/soc_camera/mx3_camera.c index 1047e3e..4bae910 100644 --- a/drivers/media/platform/soc_camera/mx3_camera.c +++ b/drivers/media/platform/soc_camera/mx3_camera.c @@ -334,7 +334,7 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb) if (!mx3_cam-active) mx3_cam-active = buf; - spin_unlock_irq(mx3_cam-lock); + spin_unlock_irqrestore(mx3_cam-lock, flags); cookie = txd-tx_submit(txd); dev_dbg(icd-parent, Submitted cookie %d DMA 0x%08x\n, @@ -343,7 +343,7 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb) if (cookie = 0) return; - spin_lock_irq(mx3_cam-lock); + spin_lock_irqsave(mx3_cam-lock, flags); /* Submit error */ list_del_init(buf-queue); -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [media] V4L: Add driver for S3C24XX/S3C64XX SoC series camera interface
[ Going through some old warnings... ]
Hello Sylwester Nawrocki,
This is a semi-automatic email about new static checker warnings.
The patch babde1c243b2: [media] V4L: Add driver for S3C24XX/S3C64XX
SoC series camera interface from Aug 22, 2012, leads to the
following Smatch complaint:
drivers/media/platform/s3c-camif/camif-capture.c:463 queue_setup()
warn: variable dereferenced before check 'fmt' (see line 460)
drivers/media/platform/s3c-camif/camif-capture.c
455 if (pfmt) {
456 pix = pfmt-fmt.pix;
457 fmt = s3c_camif_find_format(vp, pix-pixelformat, -1);
458 size = (pix-width * pix-height * fmt-depth) / 8;
^^
Dereference.
459 } else {
460 size = (frame-f_width * frame-f_height * fmt-depth)
/ 8;
^^
Dereference.
461 }
462
463 if (fmt == NULL)
^^^
Check.
464 return -EINVAL;
465 *num_planes = 1;
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] sh_vou: almost forever loop in sh_vou_try_fmt_vid_out()
The i part of the i ARRAY_SIZE() condition was missing.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/platform/sh_vou.c b/drivers/media/platform/sh_vou.c
index 7a9c5e9..41f612c 100644
--- a/drivers/media/platform/sh_vou.c
+++ b/drivers/media/platform/sh_vou.c
@@ -776,9 +776,10 @@ static int sh_vou_try_fmt_vid_out(struct file *file, void
*priv,
v4l_bound_align_image(pix-width, 0, VOU_MAX_IMAGE_WIDTH, 1,
pix-height, 0, VOU_MAX_IMAGE_HEIGHT, 1, 0);
- for (i = 0; ARRAY_SIZE(vou_fmt); i++)
+ for (i = 0; i ARRAY_SIZE(vou_fmt); i++) {
if (vou_fmt[i].pfmt == pix-pixelformat)
return 0;
+ }
pix-pixelformat = vou_fmt[0].pfmt;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media] V4L: Add driver for S3C24XX/S3C64XX SoC series camera interface
On Sun, Aug 25, 2013 at 02:23:18PM +0200, Sylwester Nawrocki wrote:
On 08/23/2013 11:46 AM, Dan Carpenter wrote:
[ Going through some old warnings... ]
Hello Sylwester Nawrocki,
This is a semi-automatic email about new static checker warnings.
The patch babde1c243b2: [media] V4L: Add driver for S3C24XX/S3C64XX
SoC series camera interface from Aug 22, 2012, leads to the
following Smatch complaint:
drivers/media/platform/s3c-camif/camif-capture.c:463 queue_setup()
warn: variable dereferenced before check 'fmt' (see line 460)
drivers/media/platform/s3c-camif/camif-capture.c
455 if (pfmt) {
456 pix =pfmt-fmt.pix;
457 fmt = s3c_camif_find_format(vp,pix-pixelformat,
-1);
458 size = (pix-width * pix-height * fmt-depth) / 8;
^^
Dereference.
459 } else {
460 size = (frame-f_width * frame-f_height *
fmt-depth) / 8;
^^
Dereference.
461 }
462
463 if (fmt == NULL)
^^^
Check.
Thanks for the bug report. This check of course should be before line 455.
Would you like to sent a patch for this or should I handle that ?
Could you handle it and give me the Reported-by tag?
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v2] [media] mx3-camera: locking cleanup in mx3_videobuf_queue()
Smatch complains about the locking here because we mix spin_lock_irq() with spin_lock_irqsave() in an unusual way. According to Smatch, it's not always clear if the IRQs are enabled or disabled when we return. It turns out this function is always called with IRQs enabled and we can just use spin_lock_irq(). It's called from __enqueue_in_driver(). Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- v2: The first version changed everything to irq_save/restore() but that wasn't right because we wanted IRQs enabled and not simply restored. diff --git a/drivers/media/platform/soc_camera/mx3_camera.c b/drivers/media/platform/soc_camera/mx3_camera.c index 1047e3e..18eab8e 100644 --- a/drivers/media/platform/soc_camera/mx3_camera.c +++ b/drivers/media/platform/soc_camera/mx3_camera.c @@ -266,7 +266,6 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb) struct idmac_channel *ichan = mx3_cam-idmac_channel[0]; struct idmac_video_param *video = ichan-params.video; const struct soc_mbus_pixelfmt *host_fmt = icd-current_fmt-host_fmt; - unsigned long flags; dma_cookie_t cookie; size_t new_size; @@ -328,7 +327,7 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb) memset(vb2_plane_vaddr(vb, 0), 0xaa, vb2_get_plane_payload(vb, 0)); #endif - spin_lock_irqsave(mx3_cam-lock, flags); + spin_lock_irq(mx3_cam-lock); list_add_tail(buf-queue, mx3_cam-capture); if (!mx3_cam-active) @@ -351,7 +350,7 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb) if (mx3_cam-active == buf) mx3_cam-active = NULL; - spin_unlock_irqrestore(mx3_cam-lock, flags); + spin_unlock_irq(mx3_cam-lock); error: vb2_buffer_done(vb, VB2_BUF_STATE_ERROR); } -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] staging: sn9c102: add a USB depend to the Kconfig
This driver won't link without USB support. Reported-by: Jim Davis jim.ep...@gmail.com Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/staging/media/sn9c102/Kconfig b/drivers/staging/media/sn9c102/Kconfig index d8ae2354b626..3ab9c81173da 100644 --- a/drivers/staging/media/sn9c102/Kconfig +++ b/drivers/staging/media/sn9c102/Kconfig @@ -1,6 +1,6 @@ config USB_SN9C102 tristate USB SN9C1xx PC Camera Controller support (DEPRECATED) - depends on VIDEO_V4L2 + depends on USB VIDEO_V4L2 ---help--- This driver is DEPRECATED, please use the gspca sonixb and sonixj modules instead. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kbuild-all] [linuxtv-media:master 499/499] drivers/media/i2c/s5k5baf.c:362:3: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t'
The other thing that concerned me with this was the sparse warning: drivers/media/i2c/s5k5baf.c:481:26: error: bad constant expression It was hard to verify that this couldn't go over 512. I guess 512 is what we would consider an error in this context. This seems like it could be determined by the firmware? regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [media] Add driver for Samsung S5K5BAF camera sensor
Hello Andrzej Hajda, This is a semi-automatic email about new static checker warnings. The patch 7d459937dc09: [media] Add driver for Samsung S5K5BAF camera sensor from Dec 5, 2013, leads to the following Smatch complaint: drivers/media/i2c/s5k5baf.c:554 s5k5baf_fw_get_seq() warn: variable dereferenced before check 'fw' (see line 551) drivers/media/i2c/s5k5baf.c 550 struct s5k5baf_fw *fw = state-fw; 551 u16 *data = fw-data + 2 * fw-count; ^ Dereference. 552 int i; 553 554 if (fw == NULL) ^^ Check. 555 return NULL; 556 regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [media] Add driver for Samsung S5K5BAF camera sensor
Hello Andrzej Hajda,
The patch 7d459937dc09: [media] Add driver for Samsung S5K5BAF
camera sensor from Dec 5, 2013, leads to the following
static checker warning:
drivers/media/i2c/s5k5baf.c:1043 s5k5baf_set_power()
warn: add some parenthesis here?
drivers/media/i2c/s5k5baf.c
1036 static int s5k5baf_set_power(struct v4l2_subdev *sd, int on)
1037 {
1038 struct s5k5baf *state = to_s5k5baf(sd);
1039 int ret = 0;
1040
1041 mutex_lock(state-lock);
1042
1043 if (!on != state-power)
^^^
This would be cleaner if it were if (on == state-power)
1044 goto out;
1045
1046 if (on) {
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media] Add driver for Samsung S5K5BAF camera sensor
On Wed, Jan 08, 2014 at 12:58:35PM +0100, Andrzej Hajda wrote:
On 01/08/2014 10:58 AM, Dan Carpenter wrote:
Hello Andrzej Hajda,
The patch 7d459937dc09: [media] Add driver for Samsung S5K5BAF
camera sensor from Dec 5, 2013, leads to the following
static checker warning:
drivers/media/i2c/s5k5baf.c:1043 s5k5baf_set_power()
warn: add some parenthesis here?
drivers/media/i2c/s5k5baf.c
1036 static int s5k5baf_set_power(struct v4l2_subdev *sd, int on)
1037 {
1038 struct s5k5baf *state = to_s5k5baf(sd);
1039 int ret = 0;
1040
1041 mutex_lock(state-lock);
1042
1043 if (!on != state-power)
^^^
This would be cleaner if it were if (on == state-power)
This version works correctly only for 'on' equal 0 and 1, my version
works for all ints. On the other side documentation says only 0 and 1 is
allowed for s_power callbacks :)
I would stay with my version, similar approach is in other drivers.
Even if (!!on == state-power) like you do in s5k5baf_s_stream() would
be more readable than the current code.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[linuxtv-media:master 499/499] drivers/media/rc/rc-main.c:1201 rc_register_device() warn: inconsistent returns mutex:dev-lock: locked (1107 [(-12)]) unlocked (1077 [(-22)], 1083 [(-22)], 1186 [0], 1
Hi Mauro,
FYI, there are new smatch warnings show up in
tree: git://linuxtv.org/media_tree.git master
head: 587d1b06e07b4a079453c74ba9edf17d21931049
commit: 587d1b06e07b4a079453c74ba9edf17d21931049 [499/499] [media] rc-core:
reuse device numbers
drivers/media/rc/rc-main.c:1201 rc_register_device() warn: inconsistent returns
mutex:dev-lock: locked (1107 [(-12)]) unlocked (1077 [(-22)], 1083 [(-22)],
1186 [0], 1201 [s32min-(-1)], 1201 [s32min-(-1),1-s32max], 1201
[s32min-(-1),1-s32max], 1201 [s32min-(-1),1-s32max])
git remote add linuxtv-media git://linuxtv.org/media_tree.git
git remote update linuxtv-media
git checkout 587d1b06e07b4a079453c74ba9edf17d21931049
vim +1201 drivers/media/rc/rc-main.c
d8b4b5822 David Härdeman 2010-10-29 1185
bc2a6c571 Mauro Carvalho Chehab 2010-11-09 1186return 0;
d8b4b5822 David Härdeman 2010-10-29 1187
d8b4b5822 David Härdeman 2010-10-29 1188 out_raw:
d8b4b5822 David Härdeman 2010-10-29 1189if (dev-driver_type ==
RC_DRIVER_IR_RAW)
d8b4b5822 David Härdeman 2010-10-29 1190
ir_raw_event_unregister(dev);
d8b4b5822 David Härdeman 2010-10-29 1191 out_input:
d8b4b5822 David Härdeman 2010-10-29 1192
input_unregister_device(dev-input_dev);
d8b4b5822 David Härdeman 2010-10-29 1193dev-input_dev = NULL;
d8b4b5822 David Härdeman 2010-10-29 1194 out_table:
b088ba658 Mauro Carvalho Chehab 2010-11-17 1195
ir_free_table(dev-rc_map);
d8b4b5822 David Härdeman 2010-10-29 1196 out_dev:
d8b4b5822 David Härdeman 2010-10-29 1197device_del(dev-dev);
08aeb7c9a Jarod Wilson 2011-05-11 1198 out_unlock:
08aeb7c9a Jarod Wilson 2011-05-11 1199
mutex_unlock(dev-lock);
587d1b06e Mauro Carvalho Chehab 2014-01-14 1200clear_bit(dev-devno,
ir_core_dev_number);
d8b4b5822 David Härdeman 2010-10-29 @1201return rc;
bc2a6c571 Mauro Carvalho Chehab 2010-11-09 1202 }
d8b4b5822 David Härdeman 2010-10-29 1203
EXPORT_SYMBOL_GPL(rc_register_device);
bc2a6c571 Mauro Carvalho Chehab 2010-11-09 1204
d8b4b5822 David Härdeman 2010-10-29 1205 void
rc_unregister_device(struct rc_dev *dev)
bc2a6c571 Mauro Carvalho Chehab 2010-11-09 1206 {
d8b4b5822 David Härdeman 2010-10-29 1207if (!dev)
d8b4b5822 David Härdeman 2010-10-29 1208return;
bc2a6c571 Mauro Carvalho Chehab 2010-11-09 1209
---
0-DAY kernel build testing backend Open Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] tda10071: remove a duplicative test
ret is an error code here, we already tested that. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/media/dvb-frontends/tda10071.c b/drivers/media/dvb-frontends/tda10071.c index 8ad3a57cf640..a76df29c4973 100644 --- a/drivers/media/dvb-frontends/tda10071.c +++ b/drivers/media/dvb-frontends/tda10071.c @@ -1006,8 +1006,7 @@ static int tda10071_init(struct dvb_frontend *fe) dev_err(priv-i2c-dev, %s: firmware \ download failed=%d\n, KBUILD_MODNAME, ret); - if (ret) - goto error_release_firmware; + goto error_release_firmware; } } release_firmware(fw); -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [media] gspca - topro: New subdriver for Topro webcams
Hello Jean-François Moine,
The patch 8f12b1ab2fac: [media] gspca - topro: New subdriver for
Topro webcams from Sep 22, 2011, leads to the following
static checker warning:
drivers/media/usb/gspca/topro.c:4642
sd_pkt_scan() warn: check 'data[]' for negative offsets s32min
drivers/media/usb/gspca/topro.c
4632 data++;
Should there be an if (len 8) return; here?
4633 len--;
4634 if (*data == 0xff data[1] == 0xd8) {
4635 /*fixme: there may be information in the 4 high bits*/
4636 if ((data[6] 0x0f) != sd-quality)
4637 set_dqt(gspca_dev, data[6] 0x0f);
4638 gspca_frame_add(gspca_dev, FIRST_PACKET,
4639 sd-jpeg_hdr, JPEG_HDR_SZ);
4640 gspca_frame_add(gspca_dev, INTER_PACKET,
4641 data + 7, len - 7);
4642 } else if (data[len - 2] == 0xff data[len - 1] ==
0xd9) {
4643 gspca_frame_add(gspca_dev, LAST_PACKET,
4644 data, len);
4645 } else {
4646 gspca_frame_add(gspca_dev, INTER_PACKET,
4647 data, len);
4648 }
4649 return;
4650 }
4651
4652 switch (*data) {
4653 case 0x55:
4654 gspca_frame_add(gspca_dev, LAST_PACKET, data, 0);
4655
4656 if (len 8
^^^
The same as there is here.
4657 || data[1] != 0xff || data[2] != 0xd8
4658 || data[3] != 0xff || data[4] != 0xfe) {
4659
4660 /* Have only seen this with corrupt frames */
4661 gspca_dev-last_packet_type = DISCARD_PACKET;
4662 return;
4663 }
4664 if (data[7] != sd-quality)
4665 set_dqt(gspca_dev, data[7]);
4666 gspca_frame_add(gspca_dev, FIRST_PACKET,
4667 sd-jpeg_hdr, JPEG_HDR_SZ);
4668 gspca_frame_add(gspca_dev, INTER_PACKET,
4669 data + 8, len - 8);
4670 break;
4671 case 0xaa:
4672 gspca_dev-last_packet_type = DISCARD_PACKET;
4673 break;
4674 case 0xcc:
I suppose we could add a if (len 1) here as well.
4675 if (data[1] != 0xff || data[2] != 0xd8)
4676 gspca_frame_add(gspca_dev, INTER_PACKET,
4677 data + 1, len - 1);
4678 else
4679 gspca_dev-last_packet_type = DISCARD_PACKET;
4680 break;
4681 }
4682 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] dvb-frontends: decimal vs hex typo in ChannelConfiguration()
From the context this should be hex 0x80 instead of decimal 80. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- Untested. diff --git a/drivers/media/dvb-frontends/tda18271c2dd.c b/drivers/media/dvb-frontends/tda18271c2dd.c index 2c54586ac07f..de0a1c110972 100644 --- a/drivers/media/dvb-frontends/tda18271c2dd.c +++ b/drivers/media/dvb-frontends/tda18271c2dd.c @@ -1030,7 +1030,7 @@ static int ChannelConfiguration(struct tda_state *state, state-m_Regs[EP4] = state-m_EP4 | state-m_IFLevelDigital; if ((Standard == HF_FM_Radio) state-m_bFMInput) - state-m_Regs[EP4] |= 80; + state-m_Regs[EP4] |= 0x80; state-m_Regs[MPD] = ~0x80; if (Standard HF_AnalogMax) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] stv0900: remove an unneeded check
No need to check lock twice here.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/dvb-frontends/stv0900_sw.c
b/drivers/media/dvb-frontends/stv0900_sw.c
index 0a40edfad739..4ce1d260b3eb 100644
--- a/drivers/media/dvb-frontends/stv0900_sw.c
+++ b/drivers/media/dvb-frontends/stv0900_sw.c
@@ -1081,7 +1081,7 @@ static int stv0900_wait_for_lock(struct stv0900_internal
*intp,
lock = stv0900_get_demod_lock(intp, demod, dmd_timeout);
if (lock)
- lock = lock stv0900_get_fec_lock(intp, demod, fec_timeout);
+ lock = stv0900_get_fec_lock(intp, demod, fec_timeout);
if (lock) {
lock = 0;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] stv090x: remove indent levels
1) We can flip the if (!lock) check to if (lock) return lock; and
then remove a big chunk of indenting.
2) There is a redundant if (!lock) which we can remove since we
already know that lock is zero. This removes another indent level.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/dvb-frontends/stv090x.c
b/drivers/media/dvb-frontends/stv090x.c
index 23e872f84742..76ee559577dd 100644
--- a/drivers/media/dvb-frontends/stv090x.c
+++ b/drivers/media/dvb-frontends/stv090x.c
@@ -2146,7 +2146,7 @@ static int stv090x_get_coldlock(struct stv090x_state
*state, s32 timeout_dmd)
u32 reg;
s32 car_step, steps, cur_step, dir, freq, timeout_lock;
- int lock = 0;
+ int lock;
if (state-srate = 1000)
timeout_lock = timeout_dmd / 3;
@@ -2154,97 +2154,96 @@ static int stv090x_get_coldlock(struct stv090x_state
*state, s32 timeout_dmd)
timeout_lock = timeout_dmd / 2;
lock = stv090x_get_dmdlock(state, timeout_lock); /* cold start wait */
- if (!lock) {
- if (state-srate = 1000) {
- if (stv090x_chk_tmg(state)) {
- if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x1f)
0)
- goto err;
- if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x15)
0)
- goto err;
- lock = stv090x_get_dmdlock(state, timeout_dmd);
- } else {
- lock = 0;
- }
+ if (lock)
+ return lock;
+
+ if (state-srate = 1000) {
+ if (stv090x_chk_tmg(state)) {
+ if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x1f) 0)
+ goto err;
+ if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x15) 0)
+ goto err;
+ lock = stv090x_get_dmdlock(state, timeout_dmd);
} else {
- if (state-srate = 400)
- car_step = 1000;
- else if (state-srate = 700)
- car_step = 2000;
- else if (state-srate = 1000)
- car_step = 3000;
+ lock = 0;
+ }
+ } else {
+ if (state-srate = 400)
+ car_step = 1000;
+ else if (state-srate = 700)
+ car_step = 2000;
+ else if (state-srate = 1000)
+ car_step = 3000;
+ else
+ car_step = 5000;
+
+ steps = (state-search_range / 1000) / car_step;
+ steps /= 2;
+ steps = 2 * (steps + 1);
+ if (steps 0)
+ steps = 2;
+ else if (steps 12)
+ steps = 12;
+
+ cur_step = 1;
+ dir = 1;
+
+ freq = state-frequency;
+ state-tuner_bw = stv090x_car_width(state-srate,
state-rolloff) + state-srate;
+ while ((cur_step = steps) (!lock)) {
+ if (dir 0)
+ freq += cur_step * car_step;
else
- car_step = 5000;
-
- steps = (state-search_range / 1000) / car_step;
- steps /= 2;
- steps = 2 * (steps + 1);
- if (steps 0)
- steps = 2;
- else if (steps 12)
- steps = 12;
-
- cur_step = 1;
- dir = 1;
-
- if (!lock) {
- freq = state-frequency;
- state-tuner_bw =
stv090x_car_width(state-srate, state-rolloff) + state-srate;
- while ((cur_step = steps) (!lock)) {
- if (dir 0)
- freq += cur_step * car_step;
- else
- freq -= cur_step * car_step;
-
- /* Setup tuner */
- if (stv090x_i2c_gate_ctrl(state, 1) 0)
- goto err;
+ freq -= cur_step * car_step;
- if (state-config-tuner_set_frequency)
{
- if
(state-config-tuner_set_frequency(fe, freq) 0)
- goto err_gateoff
[patch] [media] em28xx-cards: don't print a misleading message
There were some missing curly braces so it always says that the transfer
mode changed even if it didn't. Also the indenting uses spaces instead
of tabs.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index 4d97a76cc3b0..e8eedd35eea5 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3329,10 +3329,11 @@ static int em28xx_usb_probe(struct usb_interface
*interface,
/* Select USB transfer types to use */
if (has_video) {
- if (!dev-analog_ep_isoc || (try_bulk dev-analog_ep_bulk))
- dev-analog_xfer_bulk = 1;
- em28xx_info(analog set to %s mode.\n,
- dev-analog_xfer_bulk ? bulk : isoc);
+ if (!dev-analog_ep_isoc || (try_bulk dev-analog_ep_bulk)) {
+ dev-analog_xfer_bulk = 1;
+ em28xx_info(analog set to %s mode.\n,
+ dev-analog_xfer_bulk ? bulk : isoc);
+ }
}
if (has_dvb) {
if (!dev-dvb_ep_isoc || (try_bulk dev-dvb_ep_bulk))
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [media] stv090x: remove indent levels
On Tue, Feb 18, 2014 at 09:25:36AM +0530, Manu Abraham wrote:
Hi Dan,
On Thu, Feb 6, 2014 at 2:58 PM, Dan Carpenter dan.carpen...@oracle.com
wrote:
1) We can flip the if (!lock) check to if (lock) return lock; and
then remove a big chunk of indenting.
2) There is a redundant if (!lock) which we can remove since we
already know that lock is zero. This removes another indent level.
The stv090x driver is a mature, but slightly complex driver supporting
quite some
different configurations. Is it that some bug you are trying to fix in there ?
I wouldn't prefer unnecessary code churn in such a driver for
something as simple
as gain in an indentation level.
I thought the cleanup was jusitification enough, but the real reason I
wrote this patch is that testing:
if (!lock) {
if (!lock) {
sets off a static checker warning. That kind of code is puzzling and if
we don't clean it up then it wastes a lot of reviewer time.
Also when you're reviewing these patches please consider that the
original code might be buggy and not simply messy. Perhaps something
other than if (!lock) { was intended? When I review static checker
warnings I am looking for bugs and I don't even list cleanup patches
like this one in my status reports to my employer. Fixing these is just
something I do which saves time in the long run.
Btw, I help maintain staging so I review these kinds of patches all the
time. I use a script to review these kinds of changes. It strips out
the whitespace changes and leaves the interesting bits of the patch.
I have attached it.
cat email.patch | rename_rev.pl
regards,
dan carpenter
#!/usr/bin/perl
# This is a tool to help review variable rename patches. The goal is
# to strip out the automatic sed renames and the white space changes
# and leaves the interesting code changes.
#
# Example 1: A patch renames openInfo to open_info:
# cat diff | rename_review.pl openInfo open_info
#
# Example 2: A patch swaps the first two arguments to some_func():
# cat diff | rename_review.pl \
#-e 's/some_func\((.*?),(.*?),/some_func\($2, $1,/'
#
# Example 3: A patch removes the xkcd_ prefix from some but not all the
# variables. Instead of trying to figure out which variables were renamed
# just remove the prefix from them all:
# cat diff | rename_review.pl -ea 's/xkcd_//g'
#
# Example 4: A patch renames 20 CamelCase variables. To review this let's
# just ignore all case changes and all '_' chars.
# cat diff | rename_review -ea 'tr/[A-Z]/[a-z]/' -ea 's/_//g'
#
# The other arguments are:
# -nc removes comments
# -ns removes '\' chars if they are at the end of the line.
use strict;
use File::Temp qw/ :mktemp /;
sub usage() {
print usage: cat diff | $0 old new old new old new...\n;
printor: cat diff | $0 -e 's/old/new/g'\n;
print -e : execute on old lines\n;
print -ea: execute on all lines\n;
print -nc: no comments\n;
print -nb: no unneeded braces\n;
print -ns: no slashes at the end of a line\n;
exit(1);
}
my @subs;
my @cmds;
my $strip_comments;
my $strip_braces;
my $strip_slashes;
sub filter($) {
my $_ = shift();
my $old = 0;
if ($_ =~ /^-/) {
$old = 1;
}
# remove the first char
s/^[ +-]//;
if ($strip_comments) {
s/\/\*.*?\*\///g;
s/\/\/.*//;
}
foreach my $cmd (@cmds) {
if ($old || $cmd-[0] =~ /^-ea$/) {
eval $cmd-[1];
}
}
foreach my $sub (@subs) {
if ($old) {
s/$sub-[0]/$sub-[1]/g;
}
}
return $_;
}
while (my $param1 = shift()) {
if ($param1 =~ /^-nc$/) {
$strip_comments = 1;
next;
}
if ($param1 =~ /^-nb$/) {
$strip_braces = 1;
next;
}
if ($param1 =~ /^-ns$/) {
$strip_slashes = 1;
next;
}
my $param2 = shift();
if ($param2 =~ /^$/) {
usage();
}
if ($param1 =~ /^-e(a|)$/) {
push @cmds, [$param1, $param2];
next;
}
push @subs, [$param1, $param2];
}
my ($oldfh, $oldfile) = mkstemp(/tmp/oldX);
my ($newfh, $newfile) = mkstemp(/tmp/newX);
while () {
if ($_ =~ /^(---|\+\+\+)/) {
next;
}
my $output = filter($_);
if ($_ =~ /^-/) {
print $oldfh $output;
next;
}
if ($_ =~ /^\+/) {
print $newfh $output;
next;
}
print $oldfh $output;
print $newfh $output;
}
my $hunk;
my $old_txt;
my $new_txt;
open diff, diff -uw $oldfile $newfile |;
while (diff) {
if ($_ =~ /^(---|\+\+\+)/) {
next;
}
if ($_ =~ /^@/) {
if ($strip_comments) {
$old_txt =~ s/\/\*.*?\*\///g;
$new_txt =~ s/\/\*.*?\*\///g;
}
if ($strip_braces) {
$old_txt =~ s/{([^;{]*?);}/$1;/g;
$new_txt =~ s/{([^;{]*?);}/$1;/g;
# this is a hack because i don't know how to replace nested
# unneeded curly braces.
$old_txt =~ s/{([^;{]*?);}/$1;/g;
$new_txt =~ s/{([^;{]*?);}/$1;/g
[patch] [media] gspca_stv06xx: remove an unneeded check
err is zero here so we don't need to check again. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/media/usb/gspca/stv06xx/stv06xx_vv6410.c b/drivers/media/usb/gspca/stv06xx/stv06xx_vv6410.c index bf3e5c317a26..e60cbb3aa609 100644 --- a/drivers/media/usb/gspca/stv06xx/stv06xx_vv6410.c +++ b/drivers/media/usb/gspca/stv06xx/stv06xx_vv6410.c @@ -178,7 +178,7 @@ static int vv6410_stop(struct sd *sd) PDEBUG(D_STREAM, Halting stream); - return (err 0) ? err : 0; + return 0; } static int vv6410_dump(struct sd *sd) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [media] stv090x: remove indent levels
On Wed, Feb 19, 2014 at 10:52:32AM +0530, Manu Abraham wrote:
On Tue, Feb 18, 2014 at 2:26 PM, Dan Carpenter dan.carpen...@oracle.com
wrote:
On Tue, Feb 18, 2014 at 09:25:36AM +0530, Manu Abraham wrote:
Hi Dan,
On Thu, Feb 6, 2014 at 2:58 PM, Dan Carpenter dan.carpen...@oracle.com
wrote:
1) We can flip the if (!lock) check to if (lock) return lock; and
then remove a big chunk of indenting.
2) There is a redundant if (!lock) which we can remove since we
already know that lock is zero. This removes another indent level.
The stv090x driver is a mature, but slightly complex driver supporting
quite some
different configurations. Is it that some bug you are trying to fix in
there ?
I wouldn't prefer unnecessary code churn in such a driver for
something as simple
as gain in an indentation level.
I thought the cleanup was jusitification enough, but the real reason I
wrote this patch is that testing:
if (!lock) {
if (!lock) {
sets off a static checker warning. That kind of code is puzzling and if
we don't clean it up then it wastes a lot of reviewer time.
Also when you're reviewing these patches please consider that the
original code might be buggy and not simply messy. Perhaps something
other than if (!lock) { was intended?
I can't seem to find the possible bug in there..
The code:
lock = fn();
if (!lock) {
if (condition1) {
lock = fn()
} else {
if (!lock) {
}
}
}
looks harmless to me, AFAICS.
Yes. I thought so too. It's just a messy, but not broken. Let's just
fix the static checker warning so that we don't have to keep reviewing
it every several months.
Also, please do note that, if the
function coldlock exits due to some reason for not finding valid symbols,
stv090x_search is again fired up from the kernel frontend thread.
This sentence really scares me. Are you trying to say that the second
check on lock is valid for certain use cases? That is not possibly
true because it is a stack variable so (ignoring memory corruption)
it can't be modified from outside the code.
Hm...
The code actually looks like this:
lock = fn();
if (!lock) {
if (condition1) {
lock = fn()
} else {
if (!lock) {
while ((cur_step = steps) (!lock)) {
lock = stv090x_get_dmdlock(state, (timeout_dmd / 3));
}
}
}
}
Are you sure it's not buggy? Maybe the if statement should be moved
inside the while () condition?
It is easy to make such cleanup patches and cause breakages, but a
lot time consuming to fix such issues. My 2c.
Greg K-H and I review more of these cleanup patches than any other
kernel maintainers so I'm aware of the challenges. If you want to write
a smaller patch to fix the static checker warning then I will review it
for you. If you do that then please give me a:
Reported-by: Dan Carpenter dan.carpen...@oracle.com
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [media] stv090x: remove indent levels
Guys, what Manu is saying is purest nonsense. The lock variable is a stack variable, it's not a demodulator Read-modify-Write register. The implications of changing if (!lock) to if (lock) are simple and obvious. He's not reviewing patches, he's just NAKing them. It's not helpful. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [media] stv090x: remove indent levels
On Thu, Feb 20, 2014 at 11:24:21AM +0100, Hans Verkuil wrote: Hi Dan, This can be improved even more: Sure. Thanks. I will send v2 tomorrow. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v2] [media] stv090x: remove indent levels in stv090x_get_coldlock()
This code is needlessly complicated and checkpatch.pl complains that we
go over the 80 characters per line limit.
If we flip the if (!lock) { test to if (lock) return; then we can
remove an indent level from the rest of the function.
We can add two returns in the if (state-srate = 1000) {
condition and move the else statement back an additional indent level.
There is another if (!lock) { check which can be removed since we have
already checked lock and know it is zero at this point. This second
check on lock is also a problem because it sets off a static checker
warning. I have reviewed this code for some time to see if something
else was intended, but have concluded that it was simply an oversight
and should be removed. Removing this duplicative check gains us an
third indent level.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
v2: add the returns in the if (state-srate = 1000) { condition.
diff --git a/drivers/media/dvb-frontends/stv090x.c
b/drivers/media/dvb-frontends/stv090x.c
index 23e872f84742..93f4979ea6e9 100644
--- a/drivers/media/dvb-frontends/stv090x.c
+++ b/drivers/media/dvb-frontends/stv090x.c
@@ -2146,7 +2146,7 @@ static int stv090x_get_coldlock(struct stv090x_state
*state, s32 timeout_dmd)
u32 reg;
s32 car_step, steps, cur_step, dir, freq, timeout_lock;
- int lock = 0;
+ int lock;
if (state-srate = 1000)
timeout_lock = timeout_dmd / 3;
@@ -2154,98 +2154,96 @@ static int stv090x_get_coldlock(struct stv090x_state
*state, s32 timeout_dmd)
timeout_lock = timeout_dmd / 2;
lock = stv090x_get_dmdlock(state, timeout_lock); /* cold start wait */
- if (!lock) {
- if (state-srate = 1000) {
- if (stv090x_chk_tmg(state)) {
- if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x1f)
0)
- goto err;
- if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x15)
0)
- goto err;
- lock = stv090x_get_dmdlock(state, timeout_dmd);
- } else {
- lock = 0;
- }
- } else {
- if (state-srate = 400)
- car_step = 1000;
- else if (state-srate = 700)
- car_step = 2000;
- else if (state-srate = 1000)
- car_step = 3000;
- else
- car_step = 5000;
-
- steps = (state-search_range / 1000) / car_step;
- steps /= 2;
- steps = 2 * (steps + 1);
- if (steps 0)
- steps = 2;
- else if (steps 12)
- steps = 12;
-
- cur_step = 1;
- dir = 1;
-
- if (!lock) {
- freq = state-frequency;
- state-tuner_bw =
stv090x_car_width(state-srate, state-rolloff) + state-srate;
- while ((cur_step = steps) (!lock)) {
- if (dir 0)
- freq += cur_step * car_step;
- else
- freq -= cur_step * car_step;
-
- /* Setup tuner */
- if (stv090x_i2c_gate_ctrl(state, 1) 0)
- goto err;
+ if (lock)
+ return lock;
- if (state-config-tuner_set_frequency)
{
- if
(state-config-tuner_set_frequency(fe, freq) 0)
- goto err_gateoff;
- }
+ if (state-srate = 1000) {
+ if (stv090x_chk_tmg(state)) {
+ if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x1f) 0)
+ goto err;
+ if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x15) 0)
+ goto err;
+ return stv090x_get_dmdlock(state, timeout_dmd);
+ }
+ return 0;
+ }
- if (state-config-tuner_set_bandwidth)
{
- if
(state-config-tuner_set_bandwidth(fe, state-tuner_bw) 0)
- goto err_gateoff;
- }
+ if (state-srate = 400)
+ car_step = 1000;
+ else
Re: [patch] [media] em28xx-cards: don't print a misleading message
Thank you so much for the review. I should have noticed that myself... I will send a patch to correct the indenting instead. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] av7110_hw: fix a sanity check in av7110_fw_cmd()
ARRAY_SIZE(buf) (8 elements) was intended instead of sizeof(buf) (16
bytes). But this is just a sanity check and the callers always pass
valid values so this doesn't cause a problem.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/pci/ttpci/av7110_hw.c
b/drivers/media/pci/ttpci/av7110_hw.c
index 6299d5dadb82..300bd3c94738 100644
--- a/drivers/media/pci/ttpci/av7110_hw.c
+++ b/drivers/media/pci/ttpci/av7110_hw.c
@@ -501,7 +501,7 @@ int av7110_fw_cmd(struct av7110 *av7110, int type, int com,
int num, ...)
// dprintk(4, %p\n, av7110);
- if (2 + num sizeof(buf)) {
+ if (2 + num ARRAY_SIZE(buf)) {
printk(KERN_WARNING
%s: %s len=%d is too big!\n,
KBUILD_MODNAME, __func__, num);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] ddbridge: remove unneeded an NULL check
Static checkers complain about the inconsistent NULL check here.
There is an unchecked dereference of intput-fe in the call to
tuner_attach_tda18271() and there is a second unchecked dereference a
couple lines later when we do:
input-fe2-tuner_priv = input-fe-tuner_priv;
But actually intput-fe can't be NULL because if demod_attach_drxk()
fails to allocate it, then we would have return an error code.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c
b/drivers/media/pci/ddbridge/ddbridge-core.c
index 9375f30d9a81..fb52bda8d45f 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -876,10 +876,8 @@ static int dvb_input_attach(struct ddb_input *input)
return -ENODEV;
if (tuner_attach_tda18271(input) 0)
return -ENODEV;
- if (input-fe) {
- if (dvb_register_frontend(adap, input-fe) 0)
- return -ENODEV;
- }
+ if (dvb_register_frontend(adap, input-fe) 0)
+ return -ENODEV;
if (input-fe2) {
if (dvb_register_frontend(adap, input-fe2) 0)
return -ENODEV;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[linuxtv-media:master 463/499] drivers/media/dvb-frontends/drx39xyj/drxj.c:20803 drx_ctrl_u_code() warn: variable dereferenced before check 'mc_info' (see line 20800)
Hi Mauro,
FYI, there are new smatch warnings show up in
tree: git://linuxtv.org/media_tree.git master
head: 59432be1c7fbf2a4f608850855ff649bee0f7b3b
commit: b240eacdd536bac23c9d48dfc3d527ed6870ddad [463/499] [media] drx-j: get
rid of drx_driver.c
New smatch warnings:
drivers/media/dvb-frontends/drx39xyj/drxj.c:20803 drx_ctrl_u_code() warn:
variable dereferenced before check 'mc_info' (see line 20800)
Old smatch warnings:
drivers/media/dvb-frontends/drx39xyj/drxj.c:1218 mult32() warn: missing break?
reassigning '*h'
git remote add linuxtv-media git://linuxtv.org/media_tree.git
git remote update linuxtv-media
git checkout b240eacdd536bac23c9d48dfc3d527ed6870ddad
vim +/mc_info +20803 drivers/media/dvb-frontends/drx39xyj/drxj.c
b240eacd Mauro Carvalho Chehab 2014-01-24 20794u16 i = 0;
b240eacd Mauro Carvalho Chehab 2014-01-24 20795u16 mc_nr_of_blks = 0;
b240eacd Mauro Carvalho Chehab 2014-01-24 20796u16 mc_magic_word = 0;
b240eacd Mauro Carvalho Chehab 2014-01-24 20797const u8 *mc_data_init
= NULL;
b240eacd Mauro Carvalho Chehab 2014-01-24 20798u8 *mc_data = NULL;
b240eacd Mauro Carvalho Chehab 2014-01-24 20799unsigned size;
b240eacd Mauro Carvalho Chehab 2014-01-24 @20800char *mc_file =
mc_info-mc_file;
b240eacd Mauro Carvalho Chehab 2014-01-24 20801
b240eacd Mauro Carvalho Chehab 2014-01-24 20802/* Check arguments */
b240eacd Mauro Carvalho Chehab 2014-01-24 @20803if (!mc_info ||
!mc_file)
b240eacd Mauro Carvalho Chehab 2014-01-24 20804return -EINVAL;
b240eacd Mauro Carvalho Chehab 2014-01-24 20805
b240eacd Mauro Carvalho Chehab 2014-01-24 20806if (!demod-firmware) {
---
0-DAY kernel build testing backend Open Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[linuxtv-media:master 467/499] drivers/media/dvb-frontends/drx39xyj/drxj.c:20041 drxj_close() warn: variable dereferenced before check 'demod' (see line 20036)
Hi Mauro,
FYI, there are new smatch warnings show up in
tree: git://linuxtv.org/media_tree.git master
head: 59432be1c7fbf2a4f608850855ff649bee0f7b3b
commit: b78359a6894ac3451bec3fde5d0499fba87b8b67 [467/499] [media] drx-j: get
rid of the remaining drx generic functions
New smatch warnings:
drivers/media/dvb-frontends/drx39xyj/drxj.c:20041 drxj_close() warn: variable
dereferenced before check 'demod' (see line 20036)
Old smatch warnings:
drivers/media/dvb-frontends/drx39xyj/drxj.c:1145 mult32() warn: missing break?
reassigning '*h'
drivers/media/dvb-frontends/drx39xyj/drxj.c:20435 drx_ctrl_u_code() warn:
variable dereferenced before check 'mc_info' (see line 20432)
git remote add linuxtv-media git://linuxtv.org/media_tree.git
git remote update linuxtv-media
git checkout b78359a6894ac3451bec3fde5d0499fba87b8b67
vim +/demod +20041 drivers/media/dvb-frontends/drx39xyj/drxj.c
38b2df95 Devin Heitmueller 2012-08-13 20030 * \brief Close the demod
instance, power down the device
38b2df95 Devin Heitmueller 2012-08-13 20031 * \return Status_t Return
status.
38b2df95 Devin Heitmueller 2012-08-13 20032 *
38b2df95 Devin Heitmueller 2012-08-13 20033 */
1bfc9e15 Mauro Carvalho Chehab 2014-01-16 20034 int drxj_close(struct
drx_demod_instance *demod)
38b2df95 Devin Heitmueller 2012-08-13 20035 {
4d7bb0eb Mauro Carvalho Chehab 2014-01-16 @20036struct i2c_device_addr
*dev_addr = demod-my_i2c_dev_addr;
1bfc9e15 Mauro Carvalho Chehab 2014-01-16 20037struct drx_common_attr
*common_attr = demod-my_common_attr;
068e94ea Mauro Carvalho Chehab 2014-01-16 20038int rc;
1bfc9e15 Mauro Carvalho Chehab 2014-01-16 20039enum drx_power_mode
power_mode = DRX_POWER_UP;
38b2df95 Devin Heitmueller 2012-08-13 20040
b78359a6 Mauro Carvalho Chehab 2014-01-24 @20041if ((demod == NULL) ||
b78359a6 Mauro Carvalho Chehab 2014-01-24 20042
(demod-my_common_attr == NULL) ||
b78359a6 Mauro Carvalho Chehab 2014-01-24 20043(demod-my_ext_attr
== NULL) ||
b78359a6 Mauro Carvalho Chehab 2014-01-24 20044
(demod-my_i2c_dev_addr == NULL) ||
---
0-DAY kernel build testing backend Open Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v2] [patch] [media] em28xx-cards: remove a wrong indent level
This code is correct but the indenting is wrong and triggers a static
checker warning add curly braces?.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
v2: in v1 I added curly braces.
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index 4d97a76cc3b0..33f06ffec4b2 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3331,8 +3331,8 @@ static int em28xx_usb_probe(struct usb_interface
*interface,
if (has_video) {
if (!dev-analog_ep_isoc || (try_bulk dev-analog_ep_bulk))
dev-analog_xfer_bulk = 1;
- em28xx_info(analog set to %s mode.\n,
- dev-analog_xfer_bulk ? bulk : isoc);
+ em28xx_info(analog set to %s mode.\n,
+ dev-analog_xfer_bulk ? bulk : isoc);
}
if (has_dvb) {
if (!dev-dvb_ep_isoc || (try_bulk dev-dvb_ep_bulk))
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch v2] [patch] [media] em28xx-cards: remove a wrong indent level
On Fri, Mar 07, 2014 at 05:46:28PM +0100, Frank Schäfer wrote:
Am 05.03.2014 12:09, schrieb Dan Carpenter:
This code is correct but the indenting is wrong and triggers a static
checker warning add curly braces?.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
v2: in v1 I added curly braces.
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index 4d97a76cc3b0..33f06ffec4b2 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3331,8 +3331,8 @@ static int em28xx_usb_probe(struct usb_interface
*interface,
if (has_video) {
if (!dev-analog_ep_isoc || (try_bulk dev-analog_ep_bulk))
dev-analog_xfer_bulk = 1;
- em28xx_info(analog set to %s mode.\n,
- dev-analog_xfer_bulk ? bulk : isoc);
+ em28xx_info(analog set to %s mode.\n,
+ dev-analog_xfer_bulk ? bulk : isoc);
Instead of moving em28xx_info(...) to the left the if section needs to
be moved to the right:
- if (!dev-analog_ep_isoc || (try_bulk dev-analog_ep_bulk))
- dev-analog_xfer_bulk = 1;
+ if (!dev-analog_ep_isoc || (try_bulk dev-analog_ep_bulk))
+ dev-analog_xfer_bulk = 1;
Yes. I'd happy to.
While you are at it, could you also do fix the indention in the next
paragraph ?
The next paragraph is almost identical but my static checker was
ignoring the curly braces because of the blank line. I'll modify to
complain about that as well.
I'll resend. Thanks for the review.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] av7110: fix confusing indenting
The else statement here is not aligned with the correct if statement.
I think the code works as intended and it's just the indenting which is
wrong. Also kernel style says we should use curly braces here so I have
added those.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
This patch doesn't change how the code works, but I would still
appreciate extra review because maybe the original code is wrong?
diff --git a/drivers/media/pci/ttpci/av7110_av.c
b/drivers/media/pci/ttpci/av7110_av.c
index 301029c..9544cfc 100644
--- a/drivers/media/pci/ttpci/av7110_av.c
+++ b/drivers/media/pci/ttpci/av7110_av.c
@@ -958,8 +958,10 @@ static unsigned int dvb_video_poll(struct file *file,
poll_table *wait)
if (av7110-playing) {
if (FREE_COND)
mask |= (POLLOUT | POLLWRNORM);
- } else /* if not playing: may play if asked for */
- mask |= (POLLOUT | POLLWRNORM);
+ } else {
+ /* if not playing: may play if asked for */
+ mask |= (POLLOUT | POLLWRNORM);
+ }
}
return mask;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] dvb-core: check -msg_len for diseqc_send_master_cmd()
I'd like to send this patch except that it breaks
cx24116_send_diseqc_msg(). The cx24116 driver accepts -msg_len values
up to 24 but it looks like it's just copying 16 bytes past the end of
the -msg[] array so it's already broken.
cmd-msg_len is an unsigned char. The comment next to the struct
declaration says that valid values are are 3-6. Some of the drivers
check that this is true, but most don't and it could cause memory
corruption.
Some examples of functions which don't check are:
ttusbdecfe_dvbs_diseqc_send_master_cmd()
cx24123_send_diseqc_msg()
ds3000_send_diseqc_msg()
etc.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
Reviewed-by: Antti Palosaari cr...@iki.fi
---
This is a static checker fix and I haven't tested it but the security
implications are quite bad so we should fix this.
diff --git a/drivers/media/dvb-core/dvb_frontend.c
b/drivers/media/dvb-core/dvb_frontend.c
index 57601c0..3d1eee6 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2267,7 +2267,13 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
case FE_DISEQC_SEND_MASTER_CMD:
if (fe-ops.diseqc_send_master_cmd) {
- err = fe-ops.diseqc_send_master_cmd(fe, (struct
dvb_diseqc_master_cmd*) parg);
+ struct dvb_diseqc_master_cmd *cmd = parg;
+
+ if (cmd-msg_len = 3 cmd-msg_len = 6)
+ err = fe-ops.diseqc_send_master_cmd(fe, cmd);
+ else
+ err = -EINVAL;
+
fepriv-state = FESTATE_DISEQC;
fepriv-status = 0;
}
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [media] dvb-core: check -msg_len for diseqc_send_master_cmd()
Oops. I send this to Mauro's old email address. Sorry about that.
regards,
dan carpenter
On Tue, Apr 01, 2014 at 05:38:07PM +0300, Dan Carpenter wrote:
I'd like to send this patch except that it breaks
cx24116_send_diseqc_msg(). The cx24116 driver accepts -msg_len values
up to 24 but it looks like it's just copying 16 bytes past the end of
the -msg[] array so it's already broken.
cmd-msg_len is an unsigned char. The comment next to the struct
declaration says that valid values are are 3-6. Some of the drivers
check that this is true, but most don't and it could cause memory
corruption.
Some examples of functions which don't check are:
ttusbdecfe_dvbs_diseqc_send_master_cmd()
cx24123_send_diseqc_msg()
ds3000_send_diseqc_msg()
etc.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
Reviewed-by: Antti Palosaari cr...@iki.fi
---
This is a static checker fix and I haven't tested it but the security
implications are quite bad so we should fix this.
diff --git a/drivers/media/dvb-core/dvb_frontend.c
b/drivers/media/dvb-core/dvb_frontend.c
index 57601c0..3d1eee6 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2267,7 +2267,13 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
case FE_DISEQC_SEND_MASTER_CMD:
if (fe-ops.diseqc_send_master_cmd) {
- err = fe-ops.diseqc_send_master_cmd(fe, (struct
dvb_diseqc_master_cmd*) parg);
+ struct dvb_diseqc_master_cmd *cmd = parg;
+
+ if (cmd-msg_len = 3 cmd-msg_len = 6)
+ err = fe-ops.diseqc_send_master_cmd(fe, cmd);
+ else
+ err = -EINVAL;
+
fepriv-state = FESTATE_DISEQC;
fepriv-status = 0;
}
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: lirc: fix NULL pointer dereference
On Wed, Apr 02, 2014 at 05:18:39PM +0900, Daeseok Youn wrote:
coccicheck says:
drivers/staging/media/lirc/lirc_igorplugusb.c:226:15-21:
ERROR: ir is NULL but dereferenced.
Signed-off-by: Daeseok Youn daeseok.y...@gmail.com
---
drivers/staging/media/lirc/lirc_igorplugusb.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/lirc/lirc_igorplugusb.c
b/drivers/staging/media/lirc/lirc_igorplugusb.c
index f508a13..0ef393b 100644
--- a/drivers/staging/media/lirc/lirc_igorplugusb.c
+++ b/drivers/staging/media/lirc/lirc_igorplugusb.c
@@ -223,8 +223,8 @@ static int unregister_from_lirc(struct igorplug *ir)
int devnum;
if (!ir) {
- dev_err(ir-usbdev-dev,
- %s: called with NULL device struct!\n, __func__);
+ printk(DRIVER_NAME %s: called with NULL device struct!\n,
+__func__);
It should be pr_err() or something. But actually ir can't be NULL so
just delete the whole condition.
return -EINVAL;
}
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: lirc: remove redundant NULL check in unregister_from_lirc()
On Wed, Apr 02, 2014 at 02:49:03AM -0700, Daeseok Youn wrote: ir is already checked before calling unregister_from_lirc(). Reviewed-by: Dan Carpenter dan.carpen...@oracle.com regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media] V4L: Add driver for S3C24XX/S3C64XX SoC series camera interface
Whatever happened with this btw? Also are you sure we don't need a
second check after line 457?
regards,
dan carpenter
On Tue, Aug 27, 2013 at 04:26:07PM +0200, Sylwester Nawrocki wrote:
On 08/27/2013 04:19 PM, Dan Carpenter wrote:
On Sun, Aug 25, 2013 at 02:23:18PM +0200, Sylwester Nawrocki wrote:
On 08/23/2013 11:46 AM, Dan Carpenter wrote:
[ Going through some old warnings... ]
Hello Sylwester Nawrocki,
This is a semi-automatic email about new static checker warnings.
The patch babde1c243b2: [media] V4L: Add driver for S3C24XX/S3C64XX
SoC series camera interface from Aug 22, 2012, leads to the
following Smatch complaint:
drivers/media/platform/s3c-camif/camif-capture.c:463 queue_setup()
warn: variable dereferenced before check 'fmt' (see line 460)
drivers/media/platform/s3c-camif/camif-capture.c
455 if (pfmt) {
456 pix =pfmt-fmt.pix;
457 fmt = s3c_camif_find_format(vp,pix-pixelformat,
-1);
458 size = (pix-width * pix-height * fmt-depth) /
8;
^^
Dereference.
459} else {
460size = (frame-f_width * frame-f_height *
fmt-depth) / 8;
^^
Dereference.
461}
462
463if (fmt == NULL)
^^^
Check.
Thanks for the bug report. This check of course should be before line 455.
Would you like to sent a patch for this or should I handle that ?
Could you handle it and give me the Reported-by tag?
Sure, will do.
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] staging: media: omap24xx: fix up a checkpatch.pl warning
The two subjects are really close to being the same. You should choose better subjects. Like: [PATCH 2/2] staging: media: omap24xx: use pr_info() instead of KERN_INFO (All the checkpatch.pl people use the exact same subject for everything though, so you're not alone in this). regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] staging: media: omap24xx: fix up a checkpatch.pl warning
On Thu, Apr 10, 2014 at 09:57:23PM +1000, Vitaly Osipov wrote: Thanks, that's helpful - I for some reason thought that multi-part patch had to have more or less uniform subject. We the checkpatch.pl people come from http://www.eudyptula-challenge.org/, where at some stage we are told to submit a patch for a single style issue in the staging tree. All newbies... Hoping to be back with more substantial contributions soon. Yeah, I know about eudyptula. No worries. Newbies are welcome in staging. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media] coda: update CODA7541 to firmware 1.4.50
What ever happened with this?
regards,
dan carpenter
On Wed, Nov 06, 2013 at 07:13:43PM +0300, Dan Carpenter wrote:
Hello Philipp Zabel,
This is a semi-automatic email about new static checker warnings.
The patch 5677e3b04d3b: [media] coda: update CODA7541 to firmware
1.4.50 from Jun 21, 2013, leads to the following Smatch complaint:
drivers/media/platform/coda.c:1530 coda_alloc_framebuffers()
error: we previously assumed 'ctx-codec' could be null (see line 1521)
drivers/media/platform/coda.c
1520
1521if (ctx-codec ctx-codec-src_fourcc ==
V4L2_PIX_FMT_H264)
^^
Patch introduces a new NULL check.
1522height = round_up(height, 16);
1523ysize = round_up(q_data-width, 8) * height;
1524
1525/* Allocate frame buffers */
1526for (i = 0; i ctx-num_internal_frames; i++) {
1527size_t size;
1528
1529size = q_data-sizeimage;
1530if (ctx-codec-src_fourcc == V4L2_PIX_FMT_H264
^^
Patch introduces a new unchecked dereference.
1531dev-devtype-product != CODA_DX6)
1532ctx-internal_frames[i].size += ysize/4;
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] radio-bcm2048.c: fix wrong overflow check
From: Pali Rohár pali.ro...@gmail.com This patch fixes an off by one check in bcm2048_set_region(). Reported-by: Dan Carpenter dan.carpen...@oracle.com Signed-off-by: Pali Rohár pali.ro...@gmail.com Signed-off-by: Pavel Machek pa...@ucw.cz Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- v2: Send it to the correct list. Re-work the changelog. This patch has been floating around for four months but Pavel and Pali are knuckle-heads and don't know how to use get_maintainer.pl so they never send it to linux-media. Also Pali doesn't give reporter credit and Pavel steals authorship credit. Also when you try explain to them about how to send patches correctly they complain that they have been trying but it is too much work so now I have to do it. During the past four months thousands of other people have been able to send patches in the correct format to the correct list but it is too difficult for Pavel and Pali... *sigh*. diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c b/drivers/staging/media/bcm2048/radio-bcm2048.c index b2cd3a8..bbf236e 100644 --- a/drivers/staging/media/bcm2048/radio-bcm2048.c +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c @@ -737,7 +737,7 @@ static int bcm2048_set_region(struct bcm2048_device *bdev, u8 region) int err; u32 new_frequency = 0; - if (region ARRAY_SIZE(region_configs)) + if (region = ARRAY_SIZE(region_configs)) return -EINVAL; mutex_lock(bdev-mutex); -- -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] radio-bcm2048.c: fix wrong overflow check
On Tue, Apr 22, 2014 at 11:38:36AM +0200, Pavel Machek wrote: Feel free to resubmit the patch yourself. No problem. Happy to help. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] [media] radio-bcm2048: fix wrong overflow check
From: Pali Rohár pali.ro...@gmail.com This patch fixes an off by one check in bcm2048_set_region(). Reported-by: Dan Carpenter dan.carpen...@oracle.com Signed-off-by: Pali Rohár pali.ro...@gmail.com Signed-off-by: Pavel Machek pa...@ucw.cz Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- v2: Send it to the correct list. Re-work the changelog. v3: Correct subsystem prefix. diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c b/drivers/staging/media/bcm2048/radio-bcm2048.c index b2cd3a8..bbf236e 100644 --- a/drivers/staging/media/bcm2048/radio-bcm2048.c +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c @@ -737,7 +737,7 @@ static int bcm2048_set_region(struct bcm2048_device *bdev, u8 region) int err; u32 new_frequency = 0; - if (region ARRAY_SIZE(region_configs)) + if (region = ARRAY_SIZE(region_configs)) return -EINVAL; mutex_lock(bdev-mutex); -- -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v3] [media] em28xx-cards: fix indenting in probe()
There was a mix of 4 space and tab indenting here which was confusing.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
v3: Just fix all the surrounding indents as well.
v2: At first I thought the code was buggy and was missing curly braces
but it was just the indenting which was confusing.
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index 50aa5a5..3744766 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3416,14 +3416,14 @@ static int em28xx_usb_probe(struct usb_interface
*interface,
/* Select USB transfer types to use */
if (has_video) {
- if (!dev-analog_ep_isoc || (try_bulk dev-analog_ep_bulk))
- dev-analog_xfer_bulk = 1;
- em28xx_info(analog set to %s mode.\n,
- dev-analog_xfer_bulk ? bulk : isoc);
+ if (!dev-analog_ep_isoc || (try_bulk dev-analog_ep_bulk))
+ dev-analog_xfer_bulk = 1;
+ em28xx_info(analog set to %s mode.\n,
+ dev-analog_xfer_bulk ? bulk : isoc);
}
if (has_dvb) {
- if (!dev-dvb_ep_isoc || (try_bulk dev-dvb_ep_bulk))
- dev-dvb_xfer_bulk = 1;
+ if (!dev-dvb_ep_isoc || (try_bulk dev-dvb_ep_bulk))
+ dev-dvb_xfer_bulk = 1;
em28xx_info(dvb set to %s mode.\n,
dev-dvb_xfer_bulk ? bulk : isoc);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[linuxtv-samsung:for-v3.16 45/81] drivers/media/dvb-frontends/si2168.c:47 si2168_cmd_execute() warn: add some parenthesis here?
tree: git://linuxtv.org/snawrocki/samsung.git for-v3.16
head: 13b46c7a03adbcc347b77a13ed27066bc92d515c
commit: 192292403147877c7d5f737a3cc751ded397aef7 [45/81] [media] em28xx: add
[2013:025f] PCTV tripleStick (292e)
drivers/media/dvb-frontends/si2168.c:47 si2168_cmd_execute() warn: add some
parenthesis here?
drivers/media/dvb-frontends/si2168.c:47 si2168_cmd_execute() warn: maybe use
instead of
drivers/media/tuners/si2157.c:44 si2157_cmd_execute() warn: add some
parenthesis here?
drivers/media/tuners/si2157.c:44 si2157_cmd_execute() warn: maybe use
instead of
git remote add linuxtv-samsung git://linuxtv.org/snawrocki/samsung.git
git remote update linuxtv-samsung
git checkout 192292403147877c7d5f737a3cc751ded397aef7
vim +47 drivers/media/dvb-frontends/si2168.c
845f3505 Antti Palosaari 2014-04-10 31 goto
err_mutex_unlock;
845f3505 Antti Palosaari 2014-04-10 32 } else if (ret
!= cmd-rlen) {
845f3505 Antti Palosaari 2014-04-10 33 ret =
-EREMOTEIO;
845f3505 Antti Palosaari 2014-04-10 34 goto
err_mutex_unlock;
845f3505 Antti Palosaari 2014-04-10 35 }
845f3505 Antti Palosaari 2014-04-10 36
845f3505 Antti Palosaari 2014-04-10 37 /* firmware
ready? */
845f3505 Antti Palosaari 2014-04-10 38 if
((cmd-args[0] 7) 0x01)
845f3505 Antti Palosaari 2014-04-10 39 break;
845f3505 Antti Palosaari 2014-04-10 40 }
845f3505 Antti Palosaari 2014-04-10 41
845f3505 Antti Palosaari 2014-04-10 42
dev_dbg(s-client-dev, %s: cmd execution took %d ms\n,
845f3505 Antti Palosaari 2014-04-10 43
__func__,
845f3505 Antti Palosaari 2014-04-10 44
jiffies_to_msecs(jiffies) -
845f3505 Antti Palosaari 2014-04-10 45
(jiffies_to_msecs(timeout) - TIMEOUT));
845f3505 Antti Palosaari 2014-04-10 46
845f3505 Antti Palosaari 2014-04-10 @47 if (!(cmd-args[0]
7) 0x01) {
This should be: if (!((md-args[0]
7) 0x01)) {
Otherwise it is a precedence error where it does the negate before the
bitwise AND.
845f3505 Antti Palosaari 2014-04-10 48 ret =
-ETIMEDOUT;
845f3505 Antti Palosaari 2014-04-10 49 goto
err_mutex_unlock;
845f3505 Antti Palosaari 2014-04-10 50 }
845f3505 Antti Palosaari 2014-04-10 51 }
845f3505 Antti Palosaari 2014-04-10 52
845f3505 Antti Palosaari 2014-04-10 53 ret = 0;
845f3505 Antti Palosaari 2014-04-10 54
845f3505 Antti Palosaari 2014-04-10 55 err_mutex_unlock:
---
0-DAY kernel build testing backend Open Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: lirc: Fix sparse warnings
On Thu, May 08, 2014 at 01:11:48PM +0300, Tuomas Tynkkynen wrote:
Fix sparse warnings by adding __user and __iomem annotations where
necessary and removing certain unnecessary casts.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkky...@iki.fi
This patch adds spaces between the cast and the variable. There
shouldn't be a cast. That rule is so people remember that casting is a
high precedence operation.
Joe recently added a check for cast spacing to ./scripts/checkpatch.pl
--strict. Run your patch through ./scripts/checkpatch.pl --strict and
fix the warnings.
@@ -470,36 +471,36 @@ static long lirc_ioctl(struct file *filep, unsigned int
cmd, unsigned long arg)
switch (cmd) {
case LIRC_GET_FEATURES:
- result = put_user(features, (__u32 *) arg);
+ result = put_user(features, (__u32 __user *) arg);
arg is alway a u32 __user pointer. Do this at the start of the
function.
u32 __user *uptr = (u32 __user *)arg;
Then replace all the arg references with uptr. Btw, the difference
between __u32 and u32 is that __u32 is for code which is shared with
user space and u32 is only allowed in kernel code.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: lirc: Fix sparse warnings
On Thu, May 08, 2014 at 01:30:28PM +0300, Dan Carpenter wrote: On Thu, May 08, 2014 at 01:11:48PM +0300, Tuomas Tynkkynen wrote: Fix sparse warnings by adding __user and __iomem annotations where necessary and removing certain unnecessary casts. Signed-off-by: Tuomas Tynkkynen tuomas.tynkky...@iki.fi This patch adds spaces between the cast and the variable. There shouldn't be a cast. I meant space. There shouldn't be a space. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] staging: lirc: Fix sparse warnings
On Thu, May 08, 2014 at 02:13:17PM +0300, Tuomas Tynkkynen wrote: Fix sparse warnings by adding __user and __iomem annotations where necessary and removing certain unnecessary casts. While at it, also use u32 in place of __u32. Signed-off-by: Tuomas Tynkkynen tuomas.tynkky...@iki.fi Thanks. Reviewed-by: Dan Carpenter dan.carpen...@oracle.com Btw, don't resend this (someone will have to fix it in a later patch) but I notice that these IOCTLs are not implemented consistently. Even outside of staging we have this problem. For example lirc_rx51_ioctl(). In this function the user gets a u32. case LIRC_GET_FEATURES: - result = put_user(features, (__u32 *) arg); + result = put_user(features, uptr); if (result) return result; break; But here they get a long. case LIRC_GET_FEATURES: - result = put_user(features, (unsigned long *) arg); + result = put_user(features, uptr); break; My feeling it should always be u32 so we don't have to write a compatability layer for 32 bit applications on a 64 bit kernel. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [media] exynos4-is: Add the FIMC-IS ISP capture DMA driver
Hello Sylwester Nawrocki,
The patch 34947b8aebe3: [media] exynos4-is: Add the FIMC-IS ISP
capture DMA driver from Dec 20, 2013, leads to the following static
checker warning:
drivers/media/platform/exynos4-is/fimc-isp-video.c:415
isp_video_try_fmt_mplane()
error: XXX NULL dereference inside function. '()' '0'
drivers/media/platform/exynos4-is/fimc-isp-video.c
390 static void __isp_video_try_fmt(struct fimc_isp *isp,
391 struct v4l2_pix_format_mplane *pixm,
392 const struct fimc_fmt **fmt)
393 {
394 *fmt = fimc_isp_find_format(pixm-pixelformat, NULL, 2);
395
396 pixm-colorspace = V4L2_COLORSPACE_SRGB;
397 pixm-field = V4L2_FIELD_NONE;
398 pixm-num_planes = (*fmt)-memplanes;
399 pixm-pixelformat = (*fmt)-fourcc;
400 /*
401 * TODO: double check with the docmentation these width/height
402 * constraints are correct.
403 */
404 v4l_bound_align_image(pixm-width, FIMC_ISP_SOURCE_WIDTH_MIN,
405FIMC_ISP_SOURCE_WIDTH_MAX, 3,
406pixm-height, FIMC_ISP_SOURCE_HEIGHT_MIN,
407FIMC_ISP_SOURCE_HEIGHT_MAX, 0, 0);
408 }
409
410 static int isp_video_try_fmt_mplane(struct file *file, void *fh,
411 struct v4l2_format *f)
412 {
413 struct fimc_isp *isp = video_drvdata(file);
414
415 __isp_video_try_fmt(isp, f-fmt.pix_mp, NULL);
This will just NULL deref. What was intended?
416 return 0;
417 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] Staging: dt3155v4l: set error code on failure
We should be returning -ENOMEM here instead of success.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/staging/media/dt3155v4l/dt3155v4l.c
b/drivers/staging/media/dt3155v4l/dt3155v4l.c
index afbc2e5..178aa5b 100644
--- a/drivers/staging/media/dt3155v4l/dt3155v4l.c
+++ b/drivers/staging/media/dt3155v4l/dt3155v4l.c
@@ -907,8 +907,10 @@ dt3155_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
if (!pd)
return -ENOMEM;
pd-vdev = video_device_alloc();
- if (!pd-vdev)
+ if (!pd-vdev) {
+ err = -ENOMEM;
goto err_video_device_alloc;
+ }
*pd-vdev = dt3155_vdev;
pci_set_drvdata(pdev, pd);/* for use in dt3155_remove() */
video_set_drvdata(pd-vdev, pd); /* for use in video_fops */
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] staging: media: as102: replace custom dprintk() with dev_dbg()
On Sat, May 17, 2014 at 08:21:03PM +0300, Antti Palosaari wrote: On 05/17/2014 07:05 PM, Martin Kepplinger wrote: don't reinvent dev_dbg(). remove dprintk() in as102_drv.c. use the common kernel coding style. Signed-off-by: Martin Kepplinger mart...@posteo.de Reviewed-by: Antti Palosaari cr...@iki.fi --- this applies to next-20140516. any more suggestions? more cleanup can be done when dprintk() is completely gone. Do you have the device? I am a bit reluctant patching that driver without any testing as it has happened too many times something has gone totally broken. Looking through the log the only time I see breakage is build breakage on allyesconfig. 1ec9a35 [media] staging: as102: Add missing function argument This was a compile warning and it definitely should have been caught before the code was submitted or merged, but it wasn't something people would hit in real life. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linuxtv-samsung:for-v3.16 45/81] drivers/media/dvb-frontends/si2168.c:47 si2168_cmd_execute() warn: add some parenthesis here?
On Mon, May 05, 2014 at 11:52:46PM +0300, Antti Palosaari wrote:
845f3505 Antti Palosaari 2014-04-10 46
845f3505 Antti Palosaari 2014-04-10 @47 if (!(cmd-args[0]
7) 0x01) {
This should be: if
(!((md-args[0] 7) 0x01)) {
Otherwise it is a precedence error where it does the negate before the
bitwise AND.
That was already on my TODO list as daily media build test sparse
warned it already http://hverkuil.home.xs4all.nl/logs/Monday.log
I am waiting for media/master kernel upgrades from 3.15-rc1 as that
kernel will hang whole machine when em28xx driver used (em28xx
driver is USB bridge for those si2168 and si2157).
Wait, what? This is a one liner. I haven't understood the connection
with 3.15-rc1?
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Staging: Media: sn9c102: Fixed a pointer declaration coding style issue
On Thu, May 22, 2014 at 04:11:38PM -0700, Chaitanya wrote: Fixed the ERROR thrown off by checkpatch.pl. Put the error message here, or say what it was. Signed-off-by: Chaitanya Hazarey c...@24.io Could you change your email client so it has your last in the From: header? This patch doesn't apply. Read this: https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/email-clients.txt regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging/media/rtl2832u_sdr: fix coding style problems
On Sun, Jun 01, 2014 at 01:00:17PM -0700, Ovidiu Toader wrote: motivation: eudyptula challenge This minor patch fixes all WARNING:SPACING style warnings in rtl2832_sdr.c The new version of the file pleases checkpatch.pl when run with --ignore LONG_LINE. Signed-off-by: Ovidiu Toader o...@phas.ubc.ca Send the patch inline, not as an attachment. Read the first paragraph. https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/email-clients.txt The subject should say something about adding blank lines. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging/media/rtl2832u_sdr: fix coding style problems by adding blank lines
On Mon, Jun 02, 2014 at 12:50:35PM -0700, Ovidiu Toader wrote: On 06/02/14 03:21, Dan Carpenter wrote: Send the patch inline, not as an attachment. Read the first paragraph. https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/email-clients.txt The subject should say something about adding blank lines. Thanks for the feedback and sorry for the inconvenience. Take 2: This minor patch fixes all WARNING:SPACING style warnings in rtl2832_sdr.c The new version of the file pleases checkpatch.pl when run with --ignore LONG_LINE. Better but not quite right. You understand that the email *is* the changelog? So now it has our conversation saved in the changelog for all time. https://www.google.com/search?q=how+to+send+a+v2+patch regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] staging/media/rtl2832u_sdr: fix coding style problems by adding blank lines
On Mon, Jun 02, 2014 at 05:38:06PM -0700, Ovidiu Toader wrote: This minor patch fixes all WARNING:SPACING style warnings in rtl2832_sdr.c The new version of the file pleases checkpatch.pl when run with --ignore LONG_LINE. Looks good. Thanks. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] davinci: vpfe: dm365: remove duplicate RSZ_LPF_INT_MASK
The RSZ_LPF_INT_MASK define is cut and pasted twice so we can remove the second instance. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.h b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.h index 010fdb2..81176fb 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.h +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.h @@ -479,7 +479,6 @@ #define RSZ_TYP_Y_SHIFT0 #define RSZ_TYP_C_SHIFT1 #define RSZ_LPF_INT_MASK 0x3f -#define RSZ_LPF_INT_MASK 0x3f #define RSZ_LPF_INT_C_SHIFT6 #define RSZ_H_PHS_MASK 0x3fff #define RSZ_H_DIF_MASK 0x3fff -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] zoran: remove duplicate ZR050_MO_COMP define
The ZR050_MO_COMP define is cut and pasted twice so we can delete the
second instance.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/media/pci/zoran/zr36050.h
b/drivers/media/pci/zoran/zr36050.h
index 9f52f0c..ea083ad 100644
--- a/drivers/media/pci/zoran/zr36050.h
+++ b/drivers/media/pci/zoran/zr36050.h
@@ -126,7 +126,6 @@ struct zr36050 {
/* zr36050 mode register bits */
#define ZR050_MO_COMP0x80
-#define ZR050_MO_COMP0x80
#define ZR050_MO_ATP 0x40
#define ZR050_MO_PASS2 0x20
#define ZR050_MO_TLM 0x10
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] cx18: remove duplicate CX18_ALSA_DBGFLG_WARN define
The CX18_ALSA_DBGFLG_WARN is cut and pasted twice and we can delete the second instance. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/media/pci/cx18/cx18-alsa.h b/drivers/media/pci/cx18/cx18-alsa.h index 447da37..2718be2 100644 --- a/drivers/media/pci/cx18/cx18-alsa.h +++ b/drivers/media/pci/cx18/cx18-alsa.h @@ -49,7 +49,6 @@ static inline void snd_cx18_unlock(struct snd_cx18_card *cxsc) } #define CX18_ALSA_DBGFLG_WARN (1 0) -#define CX18_ALSA_DBGFLG_WARN (1 0) #define CX18_ALSA_DBGFLG_INFO (1 1) #define CX18_ALSA_DEBUG(x, type, fmt, args...) \ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [media] cx18: remove duplicate CX18_ALSA_DBGFLG_WARN define
Btw, the ivtv-de...@ivtvdriver.org list appears to be subscribers-only even though it says moderated in the MAINTAINERS file. It's a waste of time to list it in MAINTAINERS at that point. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [media] davinci: vpif: missing unlocks on error
We recently changed some locking around so we need some new unlocks on
the error paths.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Please review this one carefully. I don't know if the unlock should go
before or after the list_for_each_entry_safe() loop.
diff --git a/drivers/media/platform/davinci/vpif_capture.c
b/drivers/media/platform/davinci/vpif_capture.c
index a7ed164..2c08fbd 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -265,6 +265,8 @@ static int vpif_start_streaming(struct vb2_queue *vq,
unsigned int count)
return 0;
err:
+ spin_unlock_irqrestore(common-irqlock, flags);
+
list_for_each_entry_safe(buf, tmp, common-dma_queue, list) {
list_del(buf-list);
vb2_buffer_done(buf-vb, VB2_BUF_STATE_QUEUED);
diff --git a/drivers/media/platform/davinci/vpif_display.c
b/drivers/media/platform/davinci/vpif_display.c
index 5bb085b..b7b2bdf 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -229,6 +229,8 @@ static int vpif_start_streaming(struct vb2_queue *vq,
unsigned int count)
return 0;
err:
+ spin_unlock_irqrestore(common-irqlock, flags);
+
list_for_each_entry_safe(buf, tmp, common-dma_queue, list) {
list_del(buf-list);
vb2_buffer_done(buf-vb, VB2_BUF_STATE_QUEUED);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v2] [media] davinci: vpif: missing unlocks on error
We recently changed some locking around so we need some new unlocks on the error paths. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- v2: move the unlock so the list_for_each_entry_safe() loop is protected diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c index a7ed164..1e4ec69 100644 --- a/drivers/media/platform/davinci/vpif_capture.c +++ b/drivers/media/platform/davinci/vpif_capture.c @@ -269,6 +269,7 @@ err: list_del(buf-list); vb2_buffer_done(buf-vb, VB2_BUF_STATE_QUEUED); } + spin_unlock_irqrestore(common-irqlock, flags); return ret; } diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c index 5bb085b..b431b58 100644 --- a/drivers/media/platform/davinci/vpif_display.c +++ b/drivers/media/platform/davinci/vpif_display.c @@ -233,6 +233,7 @@ err: list_del(buf-list); vb2_buffer_done(buf-vb, VB2_BUF_STATE_QUEUED); } + spin_unlock_irqrestore(common-irqlock, flags); return ret; } -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [media] dvb_usb_v2: use dev_* logging macros
Hello Antti Palosaari,
This is a semi-automatic email about new static checker warnings.
The patch d10d1b9ac97b: [media] dvb_usb_v2: use dev_* logging
macros from Jun 26, 2012, leads to the following Smatch complaint:
drivers/media/usb/dvb-usb-v2/dvb_usb_urb.c:31 dvb_usb_v2_generic_io()
error: we previously assumed 'd' could be null (see line 29)
drivers/media/usb/dvb-usb-v2/dvb_usb_urb.c
28
29 if (!d || !wbuf || !wlen ||
!d-props-generic_bulk_ctrl_endpoint ||
^^
Old check.
30 !d-props-generic_bulk_ctrl_endpoint_response)
{
31 dev_dbg(d-udev-dev, %s: failed=%d\n, __func__,
-EINVAL);
^
New dereference.
32 return -EINVAL;
33 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] davinci-vpfe: Fix retcode check
On Tue, Jul 08, 2014 at 04:32:57PM +0200, Levente Kurusa wrote: 2014-07-08 16:08 GMT+02:00 Andrey Utkin andrey.krieger.ut...@gmail.com: Use signed type to check correctly for negative error code. The issue was reported with static analyser: [linux-3.13/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c:270]: (style) A pointer can not be negative so it is either pointless or an error to check if it is. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=69071 Reported-by: David Binderman dcb...@hotmail.com Signed-off-by: Andrey Utkin andrey.krieger.ut...@gmail.com Hmm, while it is true that get_ipipe_mode returns an int, but the consequent call to regw_ip takes an u32 as its second argument. Did it cause a build warning for you? It won't cause a compile warning. (Can't really check since I don't have ARM cross compilers close-by) Make a small test program and test. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] solo6x10: expose encoder quantization setting as V4L2 control
On Tue, Jul 08, 2014 at 05:30:33PM +0300, Andrey Utkin wrote: solo6*10 boards have configurable quantization parameter which takes values from 0 to 31, inclusively. This change enables setting it with ioctl VIDIOC_S_CTRL with id V4L2_CID_MPEG_VIDEO_H264_MIN_QP. Both of these two need signed-off-by lines. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: msi2500: move msi3101 out of staging and rename
Hello Antti Palosaari,
The patch fd8b5f502929: msi2500: move msi3101 out of staging and
rename from Jul 13, 2014, leads to the following static checker
warning:
drivers/media/usb/msi2500/msi2500.c:887 msi2500_stop_streaming()
error: we previously assumed 's-udev' could be null (see line 880)
drivers/media/usb/msi2500/msi2500.c
872 static void msi2500_stop_streaming(struct vb2_queue *vq)
873 {
874 struct msi2500_state *s = vb2_get_drv_priv(vq);
875
876 dev_dbg(s-udev-dev, %s:\n, __func__);
877
878 mutex_lock(s-v4l2_lock);
879
880 if (s-udev)
^^^
Check.
881 msi2500_isoc_cleanup(s);
882
883 msi2500_cleanup_queued_bufs(s);
884
885 /* according to tests, at least 700us delay is required */
886 msleep(20);
887 if (!msi2500_ctrl_msg(s, CMD_STOP_STREAMING, 0)) {
^^
Unchecked dereference if you have debugging enabled.
888 /* sleep USB IF / ADC */
889 msi2500_ctrl_msg(s, CMD_WREG, 0x0103);
890 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
