| Message ID | 20231102141135.369-2-adiupina@astralinux.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] Remove redundant return value check | expand |
Hi Alexandra On Thu, Nov 02, 2023 at 05:11:33PM +0300, Alexandra Diupina wrote: > media_entity_pads_init() will not return 0 only if the > 2nd parameter >= MEDIA_ENTITY_MAX_PADS (512), but 1 is > passed, so checking the return value is redundant I do also see if (hweight32(iter->flags & (MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE)) != 1) { ret = -EINVAL; break; } as a possible failure in in media_entity_pads_init(). > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: 34009bffc1c6 ("media: i2c: Add RDACM20 driver") And this does not really qualify as a fix imho In general, being defensive and check for return errors in not a bad thing. I would keep the check in place, it really doesn't hurt. Same for the other patches in the series. Thanks j > Signed-off-by: Alexandra Diupina <adiupina@astralinux.ru> > --- > drivers/media/i2c/rdacm20.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c > index f4e2e2f3972a..ed249ade54e0 100644 > --- a/drivers/media/i2c/rdacm20.c > +++ b/drivers/media/i2c/rdacm20.c > @@ -611,9 +611,7 @@ static int rdacm20_probe(struct i2c_client *client) > > dev->pad.flags = MEDIA_PAD_FL_SOURCE; > dev->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; > - ret = media_entity_pads_init(&dev->sd.entity, 1, &dev->pad); > - if (ret < 0) > - goto error_free_ctrls; > + media_entity_pads_init(&dev->sd.entity, 1, &dev->pad); > > ret = v4l2_async_register_subdev(&dev->sd); > if (ret) > -- > 2.30.2 >
On Thu, Nov 02, 2023 at 05:11:33PM +0300, Alexandra Diupina wrote: > media_entity_pads_init() will not return 0 only if the > 2nd parameter >= MEDIA_ENTITY_MAX_PADS (512), but 1 is > passed, so checking the return value is redundant That may be the case today, but may not be true tomorrow if the function is extended to perform extra checks. I don't think dropping the error check in drivers is a good idea. > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: 34009bffc1c6 ("media: i2c: Add RDACM20 driver") > Signed-off-by: Alexandra Diupina <adiupina@astralinux.ru> > --- > drivers/media/i2c/rdacm20.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c > index f4e2e2f3972a..ed249ade54e0 100644 > --- a/drivers/media/i2c/rdacm20.c > +++ b/drivers/media/i2c/rdacm20.c > @@ -611,9 +611,7 @@ static int rdacm20_probe(struct i2c_client *client) > > dev->pad.flags = MEDIA_PAD_FL_SOURCE; > dev->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; > - ret = media_entity_pads_init(&dev->sd.entity, 1, &dev->pad); > - if (ret < 0) > - goto error_free_ctrls; > + media_entity_pads_init(&dev->sd.entity, 1, &dev->pad); > > ret = v4l2_async_register_subdev(&dev->sd); > if (ret)
diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c index f4e2e2f3972a..ed249ade54e0 100644 --- a/drivers/media/i2c/rdacm20.c +++ b/drivers/media/i2c/rdacm20.c @@ -611,9 +611,7 @@ static int rdacm20_probe(struct i2c_client *client) dev->pad.flags = MEDIA_PAD_FL_SOURCE; dev->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; - ret = media_entity_pads_init(&dev->sd.entity, 1, &dev->pad); - if (ret < 0) - goto error_free_ctrls; + media_entity_pads_init(&dev->sd.entity, 1, &dev->pad); ret = v4l2_async_register_subdev(&dev->sd); if (ret)
media_entity_pads_init() will not return 0 only if the 2nd parameter >= MEDIA_ENTITY_MAX_PADS (512), but 1 is passed, so checking the return value is redundant Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 34009bffc1c6 ("media: i2c: Add RDACM20 driver") Signed-off-by: Alexandra Diupina <adiupina@astralinux.ru> --- drivers/media/i2c/rdacm20.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)