| Message ID | 1438853581-17154-1-git-send-email-sifan.naeem@imgtec.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9176c6657b5c313cf504d157e6d91496ee5c8708 |
| Headers | show |
On Thu, Aug 06, 2015 at 10:33:01AM +0100, Sifan Naeem wrote: > drivers/spi/spi-img-spfi.c: In function 'img_spfi_setup': > drivers/spi/spi-img-spfi.c:446: warning: 'ret' may be used > uninitialized in this function. > > Fixes: commit b03ba9e314c1 ("spi: img-spfi: fix multiple calls to request gpio") > Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com> This isn't a good commit message: > static int img_spfi_setup(struct spi_device *spi) > { > - int ret; > + int ret = -EINVAL; You're just assigning a return value so that the compiler can't tell if we've missed anything, that's often a sign of just papering over the cracks without understanding the problem - for example there may be a missing else case in some error path that the compiler was trying to tell you about where other cleanup is needed. The changelog should say what the problem was and why the change solves it sensibly.
On Thu, Aug 6, 2015 at 1:28 PM, Mark Brown <broonie@kernel.org> wrote: > On Thu, Aug 06, 2015 at 10:33:01AM +0100, Sifan Naeem wrote: >> drivers/spi/spi-img-spfi.c: In function 'img_spfi_setup': >> drivers/spi/spi-img-spfi.c:446: warning: 'ret' may be used >> uninitialized in this function. >> >> Fixes: commit b03ba9e314c1 ("spi: img-spfi: fix multiple calls to request gpio") >> Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com> > > This isn't a good commit message: > >> static int img_spfi_setup(struct spi_device *spi) >> { >> - int ret; >> + int ret = -EINVAL; > > You're just assigning a return value so that the compiler can't tell if > we've missed anything, that's often a sign of just papering over the > cracks without understanding the problem - for example there may be a > missing else case in some error path that the compiler was trying to > tell you about where other cleanup is needed. The changelog should say > what the problem was and why the change solves it sensibly. I'll bite... It's not 100% clear to me -EINVAL is the right value. Perhaps it should be 0? Or the check for gpio_is_valid() should be removed? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c index c253b2f798b4..823cbc92d1e7 100644 --- a/drivers/spi/spi-img-spfi.c +++ b/drivers/spi/spi-img-spfi.c @@ -444,7 +444,7 @@ static int img_spfi_unprepare(struct spi_master *master, static int img_spfi_setup(struct spi_device *spi) { - int ret; + int ret = -EINVAL; struct img_spfi_device_data *spfi_data = spi_get_ctldata(spi); if (!spfi_data) {
drivers/spi/spi-img-spfi.c: In function 'img_spfi_setup': drivers/spi/spi-img-spfi.c:446: warning: 'ret' may be used uninitialized in this function. Fixes: commit b03ba9e314c1 ("spi: img-spfi: fix multiple calls to request gpio") Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com> --- drivers/spi/spi-img-spfi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)