From: Tom Rix <[email protected]>
Clang static analysis reports this representative issue
atomisp-ov2722.c:920:3: warning: 3rd function call
argument is an uninitialized value
dev_err(&client->dev, "sensor_id_high = 0x%x\n", high);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
high and low are only set when ov2722_read_reg() is successful.
Reporting the high value when there is an error is not
meaningful. The later read for low is not checked. high
and low are or-ed together and checked against a non zero
value.
Remove the unneeded error reporting for high. Initialize
high and low to 0 and use the id check to determine if
the reads were successful
The later read for revision is not checked. If it
fails the old high value will be used and the revision
will be misreported.
Since the revision is only reported and not checked or
stored it is not necessary to return if the read with
successful. This makes the ret variable unnecessary
so remove it.
Signed-off-by: Tom Rix <[email protected]>
---
.../media/atomisp/i2c/atomisp-ov2722.c | 20 ++++++++-----------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index da98094d7094a..d5d099ac1b707 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -906,22 +906,17 @@ static int ov2722_get_fmt(struct v4l2_subdev *sd,
static int ov2722_detect(struct i2c_client *client)
{
struct i2c_adapter *adapter = client->adapter;
- u16 high, low;
- int ret;
+ u16 high = 0, low = 0;
u16 id;
u8 revision;
if (!i2c_check_functionality(adapter, I2C_FUNC_I2C))
return -ENODEV;
- ret = ov2722_read_reg(client, OV2722_8BIT,
- OV2722_SC_CMMN_CHIP_ID_H, &high);
- if (ret) {
- dev_err(&client->dev, "sensor_id_high = 0x%x\n", high);
- return -ENODEV;
- }
- ret = ov2722_read_reg(client, OV2722_8BIT,
- OV2722_SC_CMMN_CHIP_ID_L, &low);
+ ov2722_read_reg(client, OV2722_8BIT,
+ OV2722_SC_CMMN_CHIP_ID_H, &high);
+ ov2722_read_reg(client, OV2722_8BIT,
+ OV2722_SC_CMMN_CHIP_ID_L, &low);
id = (high << 8) | low;
if ((id != OV2722_ID) && (id != OV2720_ID)) {
@@ -929,8 +924,9 @@ static int ov2722_detect(struct i2c_client *client)
return -ENODEV;
}
- ret = ov2722_read_reg(client, OV2722_8BIT,
- OV2722_SC_CMMN_SUB_ID, &high);
+ high = 0;
+ ov2722_read_reg(client, OV2722_8BIT,
+ OV2722_SC_CMMN_SUB_ID, &high);
revision = (u8)high & 0x0f;
dev_dbg(&client->dev, "sensor_revision = 0x%x\n", revision);
--
2.26.3
I'll do a resend.
I use git send-mail, sooo not sure what went wrong.
Tom
On 3/29/22 2:06 PM, Sakari Ailus wrote:
> On Sat, Mar 26, 2022 at 12:18:53PM -0700, [email protected] wrote:
>
> Hi Tom,
>
> It seems that somehow the Content-type header of your patch is
> application/octet-stream. I.e. not text.
>
On Sat, Mar 26, 2022 at 12:18:53PM -0700, [email protected] wrote:
Hi Tom,
It seems that somehow the Content-type header of your patch is
application/octet-stream. I.e. not text.
--
Sakari Ailus
On 30/03/2022 13:33, Sakari Ailus wrote:
> Hi Tom,
>
> On Tue, Mar 29, 2022 at 04:21:20PM -0700, Tom Rix wrote:
>> I'll do a resend.
>>
>> I use git send-mail, sooo not sure what went wrong.
>
> The resent patch also has the same Content-type. I looked a bit further and
> it seems that this is very probably added by our mail system somehow: it's
> not present on the patch I received through a different route. Weird.
Well... For me, the original patch in this thread was text/plain. But
the [RESEND PATCH] was application/octet-stream, and Thunderbird shows
it as empty body with an unnamed attachment.
Tomi
Hi Tom,
On Tue, Mar 29, 2022 at 04:21:20PM -0700, Tom Rix wrote:
> I'll do a resend.
>
> I use git send-mail, sooo not sure what went wrong.
The resent patch also has the same Content-type. I looked a bit further and
it seems that this is very probably added by our mail system somehow: it's
not present on the patch I received through a different route. Weird.
git format-patch only seems to add the header sometimes based on some
obscure criteria. It's the first time I see such a problem. It'd at least
help if the Content-type would always be there but that's certainly out of
scope of the patch.
--
Regards,
Sakari Ailus