Re: [PATCH v2 1/4] driver core: Ignore 0 in dev_err_probe()

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Sat, Aug 31, 2024 at 11:53:51AM +0300, Dan Carpenter wrote:
> On Sat, Aug 31, 2024 at 11:25:54AM +0300, Dan Carpenter wrote:
> > On Thu, Aug 22, 2024 at 04:05:38PM +0300, Andy Shevchenko wrote:
> > > In the similar way, ignore 0 error code (AKA "success") in
> > > dev_err_probe(). This helps to simplify a code such as
> > > 
> > >   if (ret < 0)
> > >     return dev_err_probe(int3472->dev, ret, err_msg);
> > > 
> > >   return ret;
> > > 
> > > to
> > > 
> > >   return dev_err_probe(int3472->dev, ret, err_msg);
> > > 
> > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > This is a terrible idea because currently Smatch is able to detect about one
> > bug per month where someone unintentionally passes the wrong error variable
> > to dev_err_probe().

How many cases you know where smatch is false positive about this?

I believe the number is only a few at most, which means that you may easily
detect this still with this change being applied, i.e. "anything that
terminates function flow with code 0, passed to dev_err_probe(), is
suspicious".

So, I don't see how it prevents you from detecting that.

> Here are the stats since Jan 2023.  All these bugs are impossible to detect now.
> 
> 2024-08-12 d3bde2243d42 iio: proximity: hx9023s: Fix error code in hx9023s_property_get()

Just looked at this one.
Whoever writes something like

if (ret || foo)
	return ...ret...;

should be punished immediately. I.o.w. has nothing to do with the API.

> 2024-07-08 101e5c5c4e76 PCI: qcom: Fix missing error code in qcom_pcie_probe()
> 2024-02-22 debabbb1f272 iio: adc: ti-ads1298: Fix error code in probe()
> 2024-01-08 9c46e3a5232d iio: adc: ad7091r8: Fix error code in ad7091r8_gpio_setup()
> 2023-12-04 35ddd61cf023 platform/x86: x86-android-tablets: Fix an IS_ERR() vs NULL check in probe
> 2023-11-20 2d37b3649c41 hwrng: starfive - Fix dev_err_probe return error
> 2023-11-30 03219a3aa6c8 drm/imagination: Fix error codes in pvr_device_clk_init()
> 2023-09-07 4b2b39f9395b watchdog: marvell_gti_wdt: Fix error code in probe()
> 2023-08-24 8886e1b03669 ASoC: codecs: Fix error code in aw88261_i2c_probe()
> 2023-06-23 ad5152b85e8b leds: aw200xx: Fix error code in probe()
> 2023-07-18 86fe3e9f4c63 phy: starfive: fix error code in probe
> 2023-07-12 0b64150c3429 Input: bcm-keypad - correct dev_err_probe() error
> 2023-06-26 5fb2864cbd50 OPP: Properly propagate error along when failing to get icc_path
> 2023-06-19 02474880e8fd ASoC: max98388: fix error code in probe()
> 2023-05-25 cc5f2eb7ce11 mfd: tps6594: Fix an error code in probe()
> 2023-05-22 1ca04f21b204 remoteproc: stm32: Fix error code in stm32_rproc_parse_dt()
> 2023-05-15 46f5dd7439e3 fbdev: omapfb: panel-tpo-td043mtea1: fix error code in probe()
> 2023-05-13 3530167c6fe8 soc: qcom: icc-bwmon: fix incorrect error code passed to dev_err_probe()
> 2023-05-12 bc97139ff135 power: supply: rt9467: Fix passing zero to 'dev_err_probe'
> 2023-03-23 c4351b646123 iio: adc: ti-ads1100: fix error code in probe()
> 2023-02-27 65609d3206f7 i2c: gxp: fix an error code in probe
> 2023-02-15 76f5aaabce49 ASoC: soc-ac97: Return correct error codes

-- 
With Best Regards,
Andy Shevchenko