diff mbox

rtl2832: remove compiler warning

Message ID 20150208224422.GA22749@turing (mailing list archive)
State New, archived
Headers show

Commit Message

Luis de Bethencourt Feb. 8, 2015, 10:44 p.m. UTC
Cleaning the following compiler warning:
rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this function

Even though it could never happen since if rtl2832_rd_demod_reg () doesn't set
tmp, this line would never run because we go to err. It is still nice to avoid
compiler warnings.

Signed-off-by: Luis de Bethencourt <luis.bg@samsung.com>
---
 drivers/media/dvb-frontends/rtl2832.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Luis de Bethencourt Feb. 8, 2015, 10:46 p.m. UTC | #1
On Sun, Feb 08, 2015 at 10:44:22PM +0000, Luis de Bethencourt wrote:
> Cleaning the following compiler warning:
> rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this function
> 
> Even though it could never happen since if rtl2832_rd_demod_reg () doesn't set
> tmp, this line would never run because we go to err. It is still nice to avoid
> compiler warnings.
> 
> Signed-off-by: Luis de Bethencourt <luis.bg@samsung.com>
> ---
>  drivers/media/dvb-frontends/rtl2832.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
> index 5d2d8f4..ad36d1c 100644
> --- a/drivers/media/dvb-frontends/rtl2832.c
> +++ b/drivers/media/dvb-frontends/rtl2832.c
> @@ -685,7 +685,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, fe_status_t *status)
>  	struct rtl2832_dev *dev = fe->demodulator_priv;
>  	struct i2c_client *client = dev->client;
>  	int ret;
> -	u32 tmp;
> +	u32 tmp = 0;
>  
>  	dev_dbg(&client->dev, "\n");
>  
> -- 
> 2.1.0
> 

Hello all :)

This warning can be seen in:
http://hverkuil.home.xs4all.nl/logs/Saturday.log

Thank you Hans for the daily build and logs.

Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Antti Palosaari Feb. 10, 2015, 10:57 a.m. UTC | #2
On 02/09/2015 12:44 AM, Luis de Bethencourt wrote:
> Cleaning the following compiler warning:
> rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this function
>
> Even though it could never happen since if rtl2832_rd_demod_reg () doesn't set
> tmp, this line would never run because we go to err. It is still nice to avoid
> compiler warnings.
>
> Signed-off-by: Luis de Bethencourt <luis.bg@samsung.com>
> ---
>   drivers/media/dvb-frontends/rtl2832.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
> index 5d2d8f4..ad36d1c 100644
> --- a/drivers/media/dvb-frontends/rtl2832.c
> +++ b/drivers/media/dvb-frontends/rtl2832.c
> @@ -685,7 +685,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, fe_status_t *status)
>   	struct rtl2832_dev *dev = fe->demodulator_priv;
>   	struct i2c_client *client = dev->client;
>   	int ret;
> -	u32 tmp;
> +	u32 tmp = 0;
>
>   	dev_dbg(&client->dev, "\n");

I looked the code and I cannot see how it could used as uninitialized. 
Dunno how it could be fixed properly.

Also, I think idiom to say compiler that variable could be uninitialized 
is to store its own value. But I am fine with zero initialization too.

u32 tmp = tmp;

regards
Antti
Luis de Bethencourt Feb. 10, 2015, 2:05 p.m. UTC | #3
On Tue, Feb 10, 2015 at 12:57:24PM +0200, Antti Palosaari wrote:
> On 02/09/2015 12:44 AM, Luis de Bethencourt wrote:
> >Cleaning the following compiler warning:
> >rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this function
> >
> >Even though it could never happen since if rtl2832_rd_demod_reg () doesn't set
> >tmp, this line would never run because we go to err. It is still nice to avoid
> >compiler warnings.
> >
> >Signed-off-by: Luis de Bethencourt <luis.bg@samsung.com>
> >---
> >  drivers/media/dvb-frontends/rtl2832.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
> >index 5d2d8f4..ad36d1c 100644
> >--- a/drivers/media/dvb-frontends/rtl2832.c
> >+++ b/drivers/media/dvb-frontends/rtl2832.c
> >@@ -685,7 +685,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, fe_status_t *status)
> >  	struct rtl2832_dev *dev = fe->demodulator_priv;
> >  	struct i2c_client *client = dev->client;
> >  	int ret;
> >-	u32 tmp;
> >+	u32 tmp = 0;
> >
> >  	dev_dbg(&client->dev, "\n");
> 
> I looked the code and I cannot see how it could used as uninitialized. Dunno
> how it could be fixed properly.

Hi Antti,

I agree. If rtl2832_rd_demod_reg() in line 696 doesn't set tmp it is because
it has errored out. Which means rtl2832_read_status() itself will goto err
before the check for 'if (tmp == 11)' line that generates the warning.

I mentioned this in my commit message :)

> 
> Also, I think idiom to say compiler that variable could be uninitialized is
> to store its own value. But I am fine with zero initialization too.
> 
> u32 tmp = tmp;
> 
> regards
> Antti
> 
> -- 
> http://palosaari.fi/

If you prefer I use the 'u32 tmp = tmp;' I am happy to change my patch.
You say you are fine with zero initialization too, but I prefer offering
whatever you think is best.

Thanks for taking the time to look at this,
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mauro Carvalho Chehab Feb. 10, 2015, 11:35 p.m. UTC | #4
Em Tue, 10 Feb 2015 12:57:24 +0200
Antti Palosaari <crope@iki.fi> escreveu:

> On 02/09/2015 12:44 AM, Luis de Bethencourt wrote:
> > Cleaning the following compiler warning:
> > rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this function
> >
> > Even though it could never happen since if rtl2832_rd_demod_reg () doesn't set
> > tmp, this line would never run because we go to err. It is still nice to avoid
> > compiler warnings.
> >
> > Signed-off-by: Luis de Bethencourt <luis.bg@samsung.com>
> > ---
> >   drivers/media/dvb-frontends/rtl2832.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
> > index 5d2d8f4..ad36d1c 100644
> > --- a/drivers/media/dvb-frontends/rtl2832.c
> > +++ b/drivers/media/dvb-frontends/rtl2832.c
> > @@ -685,7 +685,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, fe_status_t *status)
> >   	struct rtl2832_dev *dev = fe->demodulator_priv;
> >   	struct i2c_client *client = dev->client;
> >   	int ret;
> > -	u32 tmp;
> > +	u32 tmp = 0;
> >
> >   	dev_dbg(&client->dev, "\n");
> 
> I looked the code and I cannot see how it could used as uninitialized. 
> Dunno how it could be fixed properly.
> 
> Also, I think idiom to say compiler that variable could be uninitialized 
> is to store its own value. But I am fine with zero initialization too.
> 
> u32 tmp = tmp;

Actually, the right way is to declare it as:

	u32 uninitialized_var(tmp)

The syntax to suppress compiler warnings depends on the compiler:

include/linux/compiler-clang.h:#define uninitialized_var(x) x = *(&(x))
include/linux/compiler-gcc.h:#define uninitialized_var(x) x = x

Also, using uninitialized_var() better documents it.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luis de Bethencourt Feb. 11, 2015, 11:13 a.m. UTC | #5
On Tue, Feb 10, 2015 at 09:35:52PM -0200, Mauro Carvalho Chehab wrote:
> Em Tue, 10 Feb 2015 12:57:24 +0200
> Antti Palosaari <crope@iki.fi> escreveu:
> 
> > On 02/09/2015 12:44 AM, Luis de Bethencourt wrote:
> > > Cleaning the following compiler warning:
> > > rtl2832.c:703:12: warning: 'tmp' may be used uninitialized in this function
> > >
> > > Even though it could never happen since if rtl2832_rd_demod_reg () doesn't set
> > > tmp, this line would never run because we go to err. It is still nice to avoid
> > > compiler warnings.
> > >
> > > Signed-off-by: Luis de Bethencourt <luis.bg@samsung.com>
> > > ---
> > >   drivers/media/dvb-frontends/rtl2832.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
> > > index 5d2d8f4..ad36d1c 100644
> > > --- a/drivers/media/dvb-frontends/rtl2832.c
> > > +++ b/drivers/media/dvb-frontends/rtl2832.c
> > > @@ -685,7 +685,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, fe_status_t *status)
> > >   	struct rtl2832_dev *dev = fe->demodulator_priv;
> > >   	struct i2c_client *client = dev->client;
> > >   	int ret;
> > > -	u32 tmp;
> > > +	u32 tmp = 0;
> > >
> > >   	dev_dbg(&client->dev, "\n");
> > 
> > I looked the code and I cannot see how it could used as uninitialized. 
> > Dunno how it could be fixed properly.
> > 
> > Also, I think idiom to say compiler that variable could be uninitialized 
> > is to store its own value. But I am fine with zero initialization too.
> > 
> > u32 tmp = tmp;
> 
> Actually, the right way is to declare it as:
> 
> 	u32 uninitialized_var(tmp)
> 
> The syntax to suppress compiler warnings depends on the compiler:
> 
> include/linux/compiler-clang.h:#define uninitialized_var(x) x = *(&(x))
> include/linux/compiler-gcc.h:#define uninitialized_var(x) x = x
> 
> Also, using uninitialized_var() better documents it.
> 
> Regards,
> Mauro

Hi Mauro,

That is a way more elegant solution. Great!
I will check out that compiler-clang header file, it's interesting.

I just sent a revised patch using this :)

Thanks,
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
index 5d2d8f4..ad36d1c 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -685,7 +685,7 @@  static int rtl2832_read_status(struct dvb_frontend *fe, fe_status_t *status)
 	struct rtl2832_dev *dev = fe->demodulator_priv;
 	struct i2c_client *client = dev->client;
 	int ret;
-	u32 tmp;
+	u32 tmp = 0;
 
 	dev_dbg(&client->dev, "\n");