| Message ID | 1fa4ab12-0939-477d-bc92-306fd32e4fd9@stanley.mountain (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: adc: pac1921: add missing error return in probe() | expand |
Le 08/08/2024 à 21:28, Dan Carpenter a écrit : > This error path was intended to return, and not just print an error. The > current code will lead to an error pointer dereference. > > Fixes: 371f778b83cd ("iio: adc: add support for pac1921") > Signed-off-by: Dan Carpenter <dan.carpenter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > --- > drivers/iio/adc/pac1921.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c > index d04c6685d780..8200a47bdf21 100644 > --- a/drivers/iio/adc/pac1921.c > +++ b/drivers/iio/adc/pac1921.c > @@ -1168,8 +1168,8 @@ static int pac1921_probe(struct i2c_client *client) > > priv->regmap = devm_regmap_init_i2c(client, &pac1921_regmap_config); > if (IS_ERR(priv->regmap)) > - dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > - "Cannot initialize register map\n"); > + return dev_err_probe(dev, (int)PTR_ERR(priv->regmap), The (int) is unusual. CJ > + "Cannot initialize register map\n"); > > devm_mutex_init(dev, &priv->lock); >
Dan Carpenter wrote: > This error path was intended to return, and not just print an error. The > current code will lead to an error pointer dereference. > > Fixes: 371f778b83cd ("iio: adc: add support for pac1921") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > drivers/iio/adc/pac1921.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c > index d04c6685d780..8200a47bdf21 100644 > --- a/drivers/iio/adc/pac1921.c > +++ b/drivers/iio/adc/pac1921.c > @@ -1168,8 +1168,8 @@ static int pac1921_probe(struct i2c_client *client) > > priv->regmap = devm_regmap_init_i2c(client, &pac1921_regmap_config); > if (IS_ERR(priv->regmap)) > - dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > - "Cannot initialize register map\n"); > + return dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > + "Cannot initialize register map\n"); > > devm_mutex_init(dev, &priv->lock); > > -- > 2.43.0 > The intent was indeed to return the error. Thanks for catching it. Acked-by: Matteo Martelli <matteomartelli3@gmail.com> Thanks, Matteo Martelli
Christophe JAILLET wrote: > Le 08/08/2024 à 21:28, Dan Carpenter a écrit : > > This error path was intended to return, and not just print an error. The > > current code will lead to an error pointer dereference. > > > > Fixes: 371f778b83cd ("iio: adc: add support for pac1921") > > Signed-off-by: Dan Carpenter <dan.carpenter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > --- > > drivers/iio/adc/pac1921.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c > > index d04c6685d780..8200a47bdf21 100644 > > --- a/drivers/iio/adc/pac1921.c > > +++ b/drivers/iio/adc/pac1921.c > > @@ -1168,8 +1168,8 @@ static int pac1921_probe(struct i2c_client *client) > > > > priv->regmap = devm_regmap_init_i2c(client, &pac1921_regmap_config); > > if (IS_ERR(priv->regmap)) > > - dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > - "Cannot initialize register map\n"); > > + return dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > The (int) is unusual. > The (int) explicit cast is to address Wconversion warnings since dev_err_probe takes an int as argument. > > CJ > > > + "Cannot initialize register map\n"); > > > > devm_mutex_init(dev, &priv->lock); > > > Thanks, Matteo Martelli
Le 09/08/2024 à 09:31, Matteo Martelli a écrit : > Christophe JAILLET wrote: >> Le 08/08/2024 à 21:28, Dan Carpenter a écrit : >>> This error path was intended to return, and not just print an error. The >>> current code will lead to an error pointer dereference. >>> >>> Fixes: 371f778b83cd ("iio: adc: add support for pac1921") >>> Signed-off-by: Dan Carpenter <dan.carpenter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> >>> --- >>> drivers/iio/adc/pac1921.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c >>> index d04c6685d780..8200a47bdf21 100644 >>> --- a/drivers/iio/adc/pac1921.c >>> +++ b/drivers/iio/adc/pac1921.c >>> @@ -1168,8 +1168,8 @@ static int pac1921_probe(struct i2c_client *client) >>> >>> priv->regmap = devm_regmap_init_i2c(client, &pac1921_regmap_config); >>> if (IS_ERR(priv->regmap)) >>> - dev_err_probe(dev, (int)PTR_ERR(priv->regmap), >>> - "Cannot initialize register map\n"); >>> + return dev_err_probe(dev, (int)PTR_ERR(priv->regmap), >> >> The (int) is unusual. >> > The (int) explicit cast is to address Wconversion warnings since dev_err_probe > takes an int as argument. Ok, but: 1) With the cast removed, on my x86_64: $ make CFLAGS="-Wconversion" drivers/iio/adc/pac1921.o doesn't generate any error. 2) $ it grep dev_err_probe.*\)PTR_ERR | wc -l 2 $ it grep dev_err_probe.*PTR_ERR | wc -l 1948 So, should the cast be needed, maybe another fix could make sense? CJ >> >> CJ >> >>> + "Cannot initialize register map\n"); >>> >>> devm_mutex_init(dev, &priv->lock); >>> >> > > Thanks, > Matteo Martelli >
On Fri, Aug 09, 2024 at 09:31:43AM +0200, Matteo Martelli wrote: > Christophe JAILLET wrote: > > Le 08/08/2024 à 21:28, Dan Carpenter a écrit : > > > This error path was intended to return, and not just print an error. The > > > current code will lead to an error pointer dereference. > > > > > > Fixes: 371f778b83cd ("iio: adc: add support for pac1921") > > > Signed-off-by: Dan Carpenter <dan.carpenter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > > --- > > > drivers/iio/adc/pac1921.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c > > > index d04c6685d780..8200a47bdf21 100644 > > > --- a/drivers/iio/adc/pac1921.c > > > +++ b/drivers/iio/adc/pac1921.c > > > @@ -1168,8 +1168,8 @@ static int pac1921_probe(struct i2c_client *client) > > > > > > priv->regmap = devm_regmap_init_i2c(client, &pac1921_regmap_config); > > > if (IS_ERR(priv->regmap)) > > > - dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > > - "Cannot initialize register map\n"); > > > + return dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > > > The (int) is unusual. > > > The (int) explicit cast is to address Wconversion warnings since dev_err_probe > takes an int as argument. I don't want to remove the int because it's unrelated, but Christophe is right that the int is unusual. We really would want to discourage that. regards, dan carpenter
Christophe JAILLET wrote: > Le 09/08/2024 à 09:31, Matteo Martelli a écrit : > > Christophe JAILLET wrote: > >> Le 08/08/2024 à 21:28, Dan Carpenter a écrit : > >>> This error path was intended to return, and not just print an error. The > >>> current code will lead to an error pointer dereference. > >>> > >>> Fixes: 371f778b83cd ("iio: adc: add support for pac1921") > >>> Signed-off-by: Dan Carpenter <dan.carpenter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > >>> --- > >>> drivers/iio/adc/pac1921.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c > >>> index d04c6685d780..8200a47bdf21 100644 > >>> --- a/drivers/iio/adc/pac1921.c > >>> +++ b/drivers/iio/adc/pac1921.c > >>> @@ -1168,8 +1168,8 @@ static int pac1921_probe(struct i2c_client *client) > >>> > >>> priv->regmap = devm_regmap_init_i2c(client, &pac1921_regmap_config); > >>> if (IS_ERR(priv->regmap)) > >>> - dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > >>> - "Cannot initialize register map\n"); > >>> + return dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > >> > >> The (int) is unusual. > >> > > The (int) explicit cast is to address Wconversion warnings since dev_err_probe > > takes an int as argument. > > Ok, but: > > 1) With the cast removed, on my x86_64: > $ make CFLAGS="-Wconversion" drivers/iio/adc/pac1921.o > > doesn't generate any error. > I can't reproduce the warning in that way either, but maybe CFLAGS gets overridden in that case because with the following method I can see the warning: $ print "CFLAGS_pac1921.o := -Wconversion" >> drivers/iio/adc/Makefile $ print "CONFIG_IIO=y\nCONFIG_PAC1921=y" >> arch/x86/configs/x86_64_defconfig $ sed -i 's/CONFIG_WERROR=y/CONFIG_WERROR=n/g' arch/x86/configs/x86_64_defconfig $ make x86_64_defconfig $ make -j7 drivers/iio/adc/pac1921.c: In function ‘pac1921_probe’: drivers/iio/adc/pac1921.c:1171:36: warning: conversion from ‘long int’ to ‘int’ may change value [-Wconversion] 1171 | dev_err_probe(dev, PTR_ERR(priv->regmap), | ^~~~~~~~~~~~~~~~~~~~~ Built with gcc version: gcc version 14.1.1 20240522 (GCC) Same thing building for aarch64 with gcc version 12.2.0 (Debian 12.2.0-14) > 2) > $ it grep dev_err_probe.*\)PTR_ERR | wc -l > 2 > > $ it grep dev_err_probe.*PTR_ERR | wc -l > 1948 > So, should the cast be needed, maybe another fix could make sense? > It could be assigned to the ret value if that would be preferred: if (IS_ERR(priv->regmap)) { ret = (int)PTR_ERR(priv->regmap); return dev_err_probe(dev, ret, "Cannot initialize register map\n"); } Otherwise a more generic approach could be to let PTR_ERR directly cast to (int). I would say that if it is always called after an IS_ERR() it should be safe to cast to (int) since the latter should guarantee the pointer value is inside int size boundaries. The similar PTR_ERR_OR_ZERO also casts (implicitly) to int but it also checks for IS_ERR before the cast. Maybe another solution could be introducing a new macro that does the cast but before it checks the ptr with IS_ERR(), I came up with the following even though it doesn't look very idiomatic: #define WITH_PTR_ERR(ret, ptr) if (IS_ERR(ptr) && (ret = (int)PTR_ERR(ptr))) ... static int pac1921_probe(struct i2c_client *client) { ... WITH_PTR_ERR(ret, priv->regmap) { return dev_err_probe(dev, ret, "Cannot initialize register map\n"); } } Maybe there is already some similar use case? Anyway, if in general it is preferred to avoid the explicit cast despite the Wconversion warning I would be fine with it. > CJ > Thanks, Matteo Martelli
Matteo Martelli wrote: > Christophe JAILLET wrote: > > Le 09/08/2024 à 09:31, Matteo Martelli a écrit : > > > Christophe JAILLET wrote: > > >> Le 08/08/2024 à 21:28, Dan Carpenter a écrit : > > >>> This error path was intended to return, and not just print an error. The > > >>> current code will lead to an error pointer dereference. > > >>> > > >>> Fixes: 371f778b83cd ("iio: adc: add support for pac1921") > > >>> Signed-off-by: Dan Carpenter <dan.carpenter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > >>> --- > > >>> drivers/iio/adc/pac1921.c | 4 ++-- > > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > > >>> > > >>> diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c > > >>> index d04c6685d780..8200a47bdf21 100644 > > >>> --- a/drivers/iio/adc/pac1921.c > > >>> +++ b/drivers/iio/adc/pac1921.c > > >>> @@ -1168,8 +1168,8 @@ static int pac1921_probe(struct i2c_client *client) > > >>> > > >>> priv->regmap = devm_regmap_init_i2c(client, &pac1921_regmap_config); > > >>> if (IS_ERR(priv->regmap)) > > >>> - dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > >>> - "Cannot initialize register map\n"); > > >>> + return dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > >> > > >> The (int) is unusual. > > >> > > > The (int) explicit cast is to address Wconversion warnings since dev_err_probe > > > takes an int as argument. > > > > Ok, but: > > > > 1) With the cast removed, on my x86_64: > > $ make CFLAGS="-Wconversion" drivers/iio/adc/pac1921.o > > > > doesn't generate any error. > > > I can't reproduce the warning in that way either, but maybe CFLAGS gets > overridden in that case because with the following method I can see the > warning: > > $ print "CFLAGS_pac1921.o := -Wconversion" >> drivers/iio/adc/Makefile > $ print "CONFIG_IIO=y\nCONFIG_PAC1921=y" >> arch/x86/configs/x86_64_defconfig > $ sed -i 's/CONFIG_WERROR=y/CONFIG_WERROR=n/g' arch/x86/configs/x86_64_defconfig > $ make x86_64_defconfig > $ make -j7 > > drivers/iio/adc/pac1921.c: In function ‘pac1921_probe’: > drivers/iio/adc/pac1921.c:1171:36: warning: conversion from ‘long int’ to ‘int’ may change value [-Wconversion] > 1171 | dev_err_probe(dev, PTR_ERR(priv->regmap), > | ^~~~~~~~~~~~~~~~~~~~~ > > Built with gcc version: gcc version 14.1.1 20240522 (GCC) > > Same thing building for aarch64 with gcc version 12.2.0 (Debian 12.2.0-14) > > > 2) > > $ it grep dev_err_probe.*\)PTR_ERR | wc -l > > 2 > > > > $ it grep dev_err_probe.*PTR_ERR | wc -l > > 1948 > > So, should the cast be needed, maybe another fix could make sense? > > > It could be assigned to the ret value if that would be preferred: > if (IS_ERR(priv->regmap)) { > ret = (int)PTR_ERR(priv->regmap); > return dev_err_probe(dev, ret, "Cannot initialize register map\n"); > } > > Otherwise a more generic approach could be to let PTR_ERR directly cast to > (int). I would say that if it is always called after an IS_ERR() it should be > safe to cast to (int) since the latter should guarantee the pointer value is > inside int size boundaries. The similar PTR_ERR_OR_ZERO also casts (implicitly) > to int but it also checks for IS_ERR before the cast. > Maybe another solution could be introducing a new macro that does the cast but > before it checks the ptr with IS_ERR(), I came up with the following even > though it doesn't look very idiomatic: > > #define WITH_PTR_ERR(ret, ptr) if (IS_ERR(ptr) && (ret = (int)PTR_ERR(ptr))) > ... > static int pac1921_probe(struct i2c_client *client) > { > ... > WITH_PTR_ERR(ret, priv->regmap) { > return dev_err_probe(dev, ret, "Cannot initialize register map\n"); > } > } > > Maybe there is already some similar use case? > > Anyway, if in general it is preferred to avoid the explicit cast despite the > Wconversion warning I would be fine with it. > Adding another simple alternative I didn't think of before: ... ret = PTR_ERR_OR_ZERO(priv->regmap); if (ret) return dev_err_probe(dev, ret, "Cannot initialize register map\n"); The warning would still be produced due to the implicit cast inside PTR_ERR_OR_ZERO but it could be fixed for all users with an explicit cast if there will be interest in future to do so. Also used a bit around: grep -R -A 3 'ret = PTR_ERR_OR_ZERO' . | grep -e '_err.*(' | wc -l 69 Thanks, Matteo Martelli
Just delete the -Wconversion? I'm not really familiar that option but it sounds an even worse version of -Wsign-compare. https://staticthinking.wordpress.com/2023/07/25/wsign-compare-is-garbage/ regards, dan carpenter
On Fri, 9 Aug 2024 18:18:13 +0300 Dan Carpenter <dan.carpenter@linaro.org> wrote: > On Fri, Aug 09, 2024 at 09:31:43AM +0200, Matteo Martelli wrote: > > Christophe JAILLET wrote: > > > Le 08/08/2024 à 21:28, Dan Carpenter a écrit : > > > > This error path was intended to return, and not just print an error. The > > > > current code will lead to an error pointer dereference. > > > > > > > > Fixes: 371f778b83cd ("iio: adc: add support for pac1921") > > > > Signed-off-by: Dan Carpenter <dan.carpenter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > > > --- > > > > drivers/iio/adc/pac1921.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c > > > > index d04c6685d780..8200a47bdf21 100644 > > > > --- a/drivers/iio/adc/pac1921.c > > > > +++ b/drivers/iio/adc/pac1921.c > > > > @@ -1168,8 +1168,8 @@ static int pac1921_probe(struct i2c_client *client) > > > > > > > > priv->regmap = devm_regmap_init_i2c(client, &pac1921_regmap_config); > > > > if (IS_ERR(priv->regmap)) > > > > - dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > > > - "Cannot initialize register map\n"); > > > > + return dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > > > > > The (int) is unusual. > > > > > The (int) explicit cast is to address Wconversion warnings since dev_err_probe > > takes an int as argument. > > I don't want to remove the int because it's unrelated, but Christophe is right > that the int is unusual. We really would want to discourage that. Applied, but I'd ideally like a follow up patch removing the int and the couple of similar instances from this driver. Anyone want to spin one? Thanks, Jonathan > > regards, > dan carpenter >
Quoting Jonathan Cameron (2024-08-10 12:35:18) > On Fri, 9 Aug 2024 18:18:13 +0300 > Dan Carpenter <dan.carpenter@linaro.org> wrote: > > > On Fri, Aug 09, 2024 at 09:31:43AM +0200, Matteo Martelli wrote: > > > Christophe JAILLET wrote: > > > > Le 08/08/2024 à 21:28, Dan Carpenter a écrit : > > > > > This error path was intended to return, and not just print an error. The > > > > > current code will lead to an error pointer dereference. > > > > > > > > > > Fixes: 371f778b83cd ("iio: adc: add support for pac1921") > > > > > Signed-off-by: Dan Carpenter <dan.carpenter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > > > > --- > > > > > drivers/iio/adc/pac1921.c | 4 ++-- > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c > > > > > index d04c6685d780..8200a47bdf21 100644 > > > > > --- a/drivers/iio/adc/pac1921.c > > > > > +++ b/drivers/iio/adc/pac1921.c > > > > > @@ -1168,8 +1168,8 @@ static int pac1921_probe(struct i2c_client *client) > > > > > > > > > > priv->regmap = devm_regmap_init_i2c(client, &pac1921_regmap_config); > > > > > if (IS_ERR(priv->regmap)) > > > > > - dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > > > > - "Cannot initialize register map\n"); > > > > > + return dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > > > > > > > The (int) is unusual. > > > > > > > The (int) explicit cast is to address Wconversion warnings since dev_err_probe > > > takes an int as argument. > > > > I don't want to remove the int because it's unrelated, but Christophe is right > > that the int is unusual. We really would want to discourage that. > > Applied, but I'd ideally like a follow up patch removing the int and the > couple of similar instances from this driver. Anyone want to spin one? > I can spin the patch, but at this point I am wondering whether I should remove all the unnecessary explicit casts that I put to address Wconversion and Wsign-compare warnings. If that's the general approach to help readability I am totally fine with it. > Thanks, > > Jonathan > Thanks, Matteo Martelli
On Wed, 11 Sep 2024 11:32:04 +0200 Matteo Martelli <matteomartelli3@gmail.com> wrote: > Quoting Jonathan Cameron (2024-08-10 12:35:18) > > On Fri, 9 Aug 2024 18:18:13 +0300 > > Dan Carpenter <dan.carpenter@linaro.org> wrote: > > > > > On Fri, Aug 09, 2024 at 09:31:43AM +0200, Matteo Martelli wrote: > > > > Christophe JAILLET wrote: > > > > > Le 08/08/2024 à 21:28, Dan Carpenter a écrit : > > > > > > This error path was intended to return, and not just print an error. The > > > > > > current code will lead to an error pointer dereference. > > > > > > > > > > > > Fixes: 371f778b83cd ("iio: adc: add support for pac1921") > > > > > > Signed-off-by: Dan Carpenter <dan.carpenter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > > > > > --- > > > > > > drivers/iio/adc/pac1921.c | 4 ++-- > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c > > > > > > index d04c6685d780..8200a47bdf21 100644 > > > > > > --- a/drivers/iio/adc/pac1921.c > > > > > > +++ b/drivers/iio/adc/pac1921.c > > > > > > @@ -1168,8 +1168,8 @@ static int pac1921_probe(struct i2c_client *client) > > > > > > > > > > > > priv->regmap = devm_regmap_init_i2c(client, &pac1921_regmap_config); > > > > > > if (IS_ERR(priv->regmap)) > > > > > > - dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > > > > > - "Cannot initialize register map\n"); > > > > > > + return dev_err_probe(dev, (int)PTR_ERR(priv->regmap), > > > > > > > > > > The (int) is unusual. > > > > > > > > > The (int) explicit cast is to address Wconversion warnings since dev_err_probe > > > > takes an int as argument. > > > > > > I don't want to remove the int because it's unrelated, but Christophe is right > > > that the int is unusual. We really would want to discourage that. > > > > Applied, but I'd ideally like a follow up patch removing the int and the > > couple of similar instances from this driver. Anyone want to spin one? > > > > I can spin the patch, but at this point I am wondering whether I should remove all > the unnecessary explicit casts that I put to address Wconversion > and Wsign-compare warnings. If that's the general approach to help readability I > am totally fine with it. I think it is probably sensible to do so as mostly we know the value ranges etc so they don't matter. Jonathan > > > Thanks, > > > > Jonathan > > > > Thanks, > Matteo Martelli
diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c index d04c6685d780..8200a47bdf21 100644 --- a/drivers/iio/adc/pac1921.c +++ b/drivers/iio/adc/pac1921.c @@ -1168,8 +1168,8 @@ static int pac1921_probe(struct i2c_client *client) priv->regmap = devm_regmap_init_i2c(client, &pac1921_regmap_config); if (IS_ERR(priv->regmap)) - dev_err_probe(dev, (int)PTR_ERR(priv->regmap), - "Cannot initialize register map\n"); + return dev_err_probe(dev, (int)PTR_ERR(priv->regmap), + "Cannot initialize register map\n"); devm_mutex_init(dev, &priv->lock);
This error path was intended to return, and not just print an error. The current code will lead to an error pointer dereference. Fixes: 371f778b83cd ("iio: adc: add support for pac1921") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- drivers/iio/adc/pac1921.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)