| Message ID | 20230713020517.614-1-duminjie@vivo.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] i2c: busses: fix parameter check in i2c_gpio_fault_injector_init() | expand |
Hi Minje, as Markus suggested, the title “i2c: gpio: Fix an error check in i2c_gpio_fault_injector_init()” looks more appropriate and... On Thu, Jul 13, 2023 at 10:05:17AM +0800, Minjie Du wrote: > Make IS_ERR() judge the debugfs_create_dir() function return. ... I think you should fix a bit the log here (also Mark has suggested it). How about something like: debugfs_create_dir() function returns an error value embedded in the pointer (PTR_ERR). We need to evaluate the return value using IS_ERR, rather than checking for NULL. > Fixes: 14911c6f48ec ("i2c: gpio: add fault injector") > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> > Cc: <stable@vger.kernel.org> # v4.16+ > don't leave space between tags. > Signed-off-by: Minjie Du <duminjie@vivo.com> > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> At the end it can be something like: i2c: gpio: Fix an error check in i2c_gpio_fault_injector_init() debugfs_create_dir() function returns an error value embedded in the pointer (PTR_ERR). Evaluate the return value using IS_ERR rather than checking for NULL. Fixes: 14911c6f48ec ("i2c: gpio: add fault injector") Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> Cc: <stable@vger.kernel.org> # v4.16+ Signed-off-by: Minjie Du <duminjie@vivo.com> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> I'm not sure whether Wolfram can fix it or, if you don't feel lazy, you can send a v4. > --- > v1-v2: > Fix judge typo. Please update also the changelog, v2-v3 and v3-v4. Thanks, Markus, for the suggestions. Andi > --- > drivers/i2c/busses/i2c-gpio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c > index e5a5b9e8b..545927b96 100644 > --- a/drivers/i2c/busses/i2c-gpio.c > +++ b/drivers/i2c/busses/i2c-gpio.c > @@ -265,7 +265,7 @@ static void i2c_gpio_fault_injector_init(struct platform_device *pdev) > */ > if (!i2c_gpio_debug_dir) { > i2c_gpio_debug_dir = debugfs_create_dir("i2c-fault-injector", NULL); > - if (!i2c_gpio_debug_dir) > + if (IS_ERR(i2c_gpio_debug_dir)) > return; > } > > -- > 2.39.0 >
Hi Markus, > > +++ b/drivers/i2c/busses/i2c-gpio.c > > @@ -265,7 +265,7 @@ static void i2c_gpio_fault_injector_init(struct platform_device *pdev) > > */ > > if (!i2c_gpio_debug_dir) { > > i2c_gpio_debug_dir = debugfs_create_dir("i2c-fault-injector", NULL); > > - if (!i2c_gpio_debug_dir) > > + if (IS_ERR(i2c_gpio_debug_dir)) > > return; > > } > > Will the development attention grow also for the exception handling > which is missing here so far? Well, it depends why you need to handle the error return. I think it's not important if this fails... the only reason I would handle the error here is to print a warning in dmesg. Thank you, Andi
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c index e5a5b9e8b..545927b96 100644 --- a/drivers/i2c/busses/i2c-gpio.c +++ b/drivers/i2c/busses/i2c-gpio.c @@ -265,7 +265,7 @@ static void i2c_gpio_fault_injector_init(struct platform_device *pdev) */ if (!i2c_gpio_debug_dir) { i2c_gpio_debug_dir = debugfs_create_dir("i2c-fault-injector", NULL); - if (!i2c_gpio_debug_dir) + if (IS_ERR(i2c_gpio_debug_dir)) return; }
Make IS_ERR() judge the debugfs_create_dir() function return. Fixes: 14911c6f48ec ("i2c: gpio: add fault injector") Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> Cc: <stable@vger.kernel.org> # v4.16+ Signed-off-by: Minjie Du <duminjie@vivo.com> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> --- v1-v2: Fix judge typo. --- drivers/i2c/busses/i2c-gpio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)