On Sat, 2009-04-04 at 14:26 +0200, Jean Delvare wrote:
> * Return actual error values as returned by the i2c subsystem, rather
> than 0 or 1.
> * If the registration of the second bus fails, unregister the first one
> before exiting, otherwise we are leaking resources.
>
> Signed-off-by: Jean Delvare <khali@xxxxxxxxxxxx>
> Cc: Hans Verkuil <hverkuil@xxxxxxxxx>
> Cc: Andy Walls <awalls@xxxxxxxxx>
Jean,
Thanks for noticing this one and providing a patch. I have one comment
below...
> ---
> linux/drivers/media/video/cx18/cx18-i2c.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> --- v4l-dvb.orig/linux/drivers/media/video/cx18/cx18-i2c.c 2009-03-01 16:09:09.000000000 +0100
> +++ v4l-dvb/linux/drivers/media/video/cx18/cx18-i2c.c 2009-04-03 18:45:18.000000000 +0200
> @@ -214,7 +214,7 @@ static struct i2c_algo_bit_data cx18_i2c
> /* init + register i2c algo-bit adapter */
> int init_cx18_i2c(struct cx18 *cx)
> {
> - int i;
> + int i, err;
> CX18_DEBUG_I2C("i2c init\n");
>
> for (i = 0; i < 2; i++) {
> @@ -273,8 +273,18 @@ int init_cx18_i2c(struct cx18 *cx)
> cx18_call_hw(cx, CX18_HW_GPIO_RESET_CTRL,
> core, reset, (u32) CX18_GPIO_RESET_I2C);
>
> - return i2c_bit_add_bus(&cx->i2c_adap[0]) ||
> - i2c_bit_add_bus(&cx->i2c_adap[1]);
> + err = i2c_bit_add_bus(&cx->i2c_adap[0]);
if (err)
return err;
err = i2c_bit_add_bus(&cx->i2c_adap[1]);
if (err)
i2c_del_adapter(&cx->i2c_adap[0]);
return err;
This sequence saves a few lines of code and gets rid of the goto's
compared to what you proposed below.
> + if (err)
> + goto err;
> + err = i2c_bit_add_bus(&cx->i2c_adap[1]);
> + if (err)
> + goto err_del_bus_0;
> + return 0;
> +
> + err_del_bus_0:
> + i2c_del_adapter(&cx->i2c_adap[0]);
> + err:
> + return err;
> }
>
> void exit_cx18_i2c(struct cx18 *cx)
Reviewed-by: Andy Walls <awalls@xxxxxxxxx>
Regards,
Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
