hwmon: (ads7871) Fix ads7871_probe error path
PATCH v2 1/1 hwmon: (ads7871) Fix ads7871_probe error path
This patch moves hwmon_device_register() to the end of ads7871_probe(). So we can make sure the device exists and is properly initialized before the device is exposed to user-space.
--- drivers/hwmon/ads7871.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
drivers/hwmon/ads7871.c +6 -6
@@ -176,12 +176,6 @@ static int __devinit ads7871_probe(struct spi_device *spi)if (status <0 ) goto error_free; - pdata->hwmon_dev =hwmon_device_register (&spi->dev); -if (IS_ERR (pdata->hwmon_dev)) { - err =PTR_ERR (pdata->hwmon_dev); - goto error_remove; - } -spi_set_drvdata (spi, pdata);/* Configure the SPI bus */
@@ -204,6 +198,12 @@ static int __devinit ads7871_probe(struct spi_device *spi) goto error_remove; } + pdata->hwmon_dev =hwmon_device_register (&spi->dev); +if (IS_ERR (pdata->hwmon_dev)) { + err =PTR_ERR (pdata->hwmon_dev); + goto error_remove; + } +return 0 ; error_remove:
Signature · 1 line
1.7.0.4
Jean Delvare
AppliedQuoted 29 lines · Axel Lin
This patch moves
hwmon_device_register()to the end ofads7871_probe(). So we can make sure the device exists and is properly initialized before the device is exposed to user-space.Signed-off-by Axel Lin <[email protected]>--- drivers/hwmon/ads7871.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)drivers/hwmon/ads7871.c +0
-6@@ -176,12 +176,6 @@ static int __devinit ads7871_probe(struct spi_device *spi)if (status <0 ) goto error_free; - pdata->hwmon_dev =hwmon_device_register (&spi->dev); -if (IS_ERR (pdata->hwmon_dev)) { - err =PTR_ERR (pdata->hwmon_dev); - goto error_remove; - } -spi_set_drvdata (spi, pdata);/* Configure the SPI bus */ @@ -204,6 +198,12 @@ static int __devinit ads7871_probe(struct spi_device *spi) goto error_remove; }
drivers/hwmon/ads7871.c static int __devinit ads7871_probe(struct spi_device *spi) · line 209
Quoted 10 lines
drivers/hwmon/ads7871.c +6
-0+ pdata->hwmon_dev =hwmon_device_register (&spi->dev); +if (IS_ERR (pdata->hwmon_dev)) { + err =PTR_ERR (pdata->hwmon_dev); + goto error_remove; + } +return 0 ; error_remove:
Applied, thanks.
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Axel, Paul,
Quoted 39 lines · Jean Delvare
Quoted 38 lines · Axel Lin
This patch moves
hwmon_device_register()to the end ofads7871_probe(). So we can make sure the device exists and is properly initialized before the device is exposed to user-space.Signed-off-by Axel Lin <[email protected]>--- drivers/hwmon/ads7871.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)drivers/hwmon/ads7871.c +6
-6@@ -176,12 +176,6 @@ static int __devinit ads7871_probe(struct spi_device *spi)if (status <0 ) goto error_free; - pdata->hwmon_dev =hwmon_device_register (&spi->dev); -if (IS_ERR (pdata->hwmon_dev)) { - err =PTR_ERR (pdata->hwmon_dev); - goto error_remove; - } -spi_set_drvdata (spi, pdata);/* Configure the SPI bus */ @@ -204,6 +198,12 @@ static int __devinit ads7871_probe(struct spi_device *spi) goto error_remove; } + pdata->hwmon_dev =hwmon_device_register (&spi->dev); +if (IS_ERR (pdata->hwmon_dev)) { + err =PTR_ERR (pdata->hwmon_dev); + goto error_remove; + } +return 0 ;
Quoted 3 lines
drivers/hwmon/ads7871.c static int __devinit ads7871_probe(struct spi_device *spi) · line 209
Quoted 1 line
drivers/hwmon/ads7871.c +0
-0error_remove:Applied, thanks.
Hmm, looking at the code again as I was about to send the patch to Linus, it seems that the above fix isn't sufficient. One problem is that it makes little sense to create sysfs attributes for the device if the probe is going to fail. So sysfs file creation should be delayed until after the device presence check. A second, more important problem is that no error code is returned if sysfs_create_group() fails. The probe function will return success (0) while the device is not registered, which means trouble when the remove function is called later.
The second problem is caused by the presence of two local variables carrying error values: err and status. The latter should be dropped to avoid the confusion, and the former shouldn't be initialized to 0, so that the compiler has a chance to warn about such issues.
Axel, Paul, can either of you please send an updated patch as a replacement for this one, fixing the two remaining issues? Thanks.
As a side note, I also noticed that struct ads7871_data has an unused member "update_lock". Someone familiar with the device and driver should check whether it should be used somewhere, or if it can be dropped.
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors