Hello everybody,
While looking into Coverity ID 1226903 I ran into the following piece
of code at drivers/media/pci/cx25821/cx25821-medusa-video.c:393:
393int medusa_set_videostandard(struct cx25821_dev *dev)
394{
395 int status = 0;
396 u32 value = 0, tmp = 0;
397
398 if (dev->tvnorm & V4L2_STD_PAL_BG || dev->tvnorm & V4L2_STD_PAL_DK)
399 status = medusa_initialize_pal(dev);
400 else
401 status = medusa_initialize_ntsc(dev);
402
403 /* Enable DENC_A output */
404 value = cx25821_i2c_read(&dev->i2c_bus[0], DENC_A_REG_4, &tmp);
405 value = setBitAtPos(value, 4);
406 status = cx25821_i2c_write(&dev->i2c_bus[0], DENC_A_REG_4, value);
407
408 /* Enable DENC_B output */
409 value = cx25821_i2c_read(&dev->i2c_bus[0], DENC_B_REG_4, &tmp);
410 value = setBitAtPos(value, 4);
411 status = cx25821_i2c_write(&dev->i2c_bus[0], DENC_B_REG_4, value);
412
413 return status;
414}
The issue is that the value stored in variable _status_ at lines 399
and 401 is overwritten by the one stored at line 406 and then at line
411, before it can be used.
My question is if the original intention was to ORed the return
values, something like in the following patch:
index 0a9db05..226d14f 100644
--- a/drivers/media/pci/cx25821/cx25821-medusa-video.c
+++ b/drivers/media/pci/cx25821/cx25821-medusa-video.c
@@ -403,12 +403,12 @@ int medusa_set_videostandard(struct cx25821_dev *dev)
/* Enable DENC_A output */
value = cx25821_i2c_read(&dev->i2c_bus[0], DENC_A_REG_4, &tmp);
value = setBitAtPos(value, 4);
- status = cx25821_i2c_write(&dev->i2c_bus[0], DENC_A_REG_4, value);
+ status |= cx25821_i2c_write(&dev->i2c_bus[0], DENC_A_REG_4, value);
/* Enable DENC_B output */
value = cx25821_i2c_read(&dev->i2c_bus[0], DENC_B_REG_4, &tmp);
value = setBitAtPos(value, 4);
- status = cx25821_i2c_write(&dev->i2c_bus[0], DENC_B_REG_4, value);
+ status |= cx25821_i2c_write(&dev->i2c_bus[0], DENC_B_REG_4, value);
return status;
}
What do you think?
I'd really appreciate any comment on this.
Thank you!
--
Gustavo A. R. Silva
