[4/8] drivers:tmp.c Fix warning: variable 'rc' set but not used

Message ID 1276547208-26569-5-git-send-email-justinmattock@gmail.com (mailing list archive)
State Not Applicable, archived
Headers

Commit Message

Justin P. Mattock June 14, 2010, 8:26 p.m. UTC
  Im getting this warning when compiling:
 CC      drivers/char/tpm/tpm.o
drivers/char/tpm/tpm.c: In function 'tpm_gen_interrupt':
drivers/char/tpm/tpm.c:508:10: warning: variable 'rc' set but not used

The below patch gets rid of the warning,
but I'm not sure if it's the best solution.

 Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

---
 drivers/char/tpm/tpm.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
  

Comments

Valdis.Kletnieks@vt.edu June 15, 2010, 12:13 a.m. UTC | #1
On Mon, 14 Jun 2010 13:26:44 PDT, "Justin P. Mattock" said:
> Im getting this warning when compiling:
>  CC      drivers/char/tpm/tpm.o
> drivers/char/tpm/tpm.c: In function 'tpm_gen_interrupt':
> drivers/char/tpm/tpm.c:508:10: warning: variable 'rc' set but not used
> 
> The below patch gets rid of the warning,
> but I'm not sure if it's the best solution.

>  	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
>  			"attempting to determine the timeouts");
> +	if (!rc)
> +		rc = 0;
>  }

Good thing that's a void function. ;)

Unless transmit_cmd() is marked 'must_check', maybe losing the 'rc =' would
be a better solution?
  
Justin P. Mattock June 15, 2010, 2:12 a.m. UTC | #2
On 06/14/2010 05:13 PM, Valdis.Kletnieks@vt.edu wrote:
> On Mon, 14 Jun 2010 13:26:44 PDT, "Justin P. Mattock" said:
>> Im getting this warning when compiling:
>>   CC      drivers/char/tpm/tpm.o
>> drivers/char/tpm/tpm.c: In function 'tpm_gen_interrupt':
>> drivers/char/tpm/tpm.c:508:10: warning: variable 'rc' set but not used
>>
>> The below patch gets rid of the warning,
>> but I'm not sure if it's the best solution.
>
>>   	rc = transmit_cmd(chip,&tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
>>   			"attempting to determine the timeouts");
>> +	if (!rc)
>> +		rc = 0;
>>   }
>
> Good thing that's a void function. ;)
>
> Unless transmit_cmd() is marked 'must_check', maybe losing the 'rc =' would
> be a better solution?


what I tried was this:

if (!rc)
	printk("test........"\n")

and everything looked good,
but as a soon as I changed

rc = transmit_cmd(chip,&tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
    			"attempting to determine the timeouts");

to this:

rc = transmit_cmd(chip,&tpm_cmd, TPM_INTERNAL_RESULT_SIZE);

if (!rc)
	printk("attempting to determine the timeouts\n");

I error out with transmit_cmd not having enough
functions to it.. so I just added the rc = 0;
and went on to the next.

Justin P. Mattock
--
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
  
Valdis.Kletnieks@vt.edu June 15, 2010, 3:49 a.m. UTC | #3
On Mon, 14 Jun 2010 19:12:31 PDT, "Justin P. Mattock" said:

> what I tried was this:
> 
> if (!rc)
> 	printk("test........"\n")
> 
> and everything looked good,
> but as a soon as I changed
> 
> rc = transmit_cmd(chip,&tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
>     			"attempting to determine the timeouts");
> 
> to this:
> 
> rc = transmit_cmd(chip,&tpm_cmd, TPM_INTERNAL_RESULT_SIZE);
> 
> if (!rc)
> 	printk("attempting to determine the timeouts\n");

*baffled* Why did you think that would work? transmit_cmd()s signature
has 4 parameters.
  
Justin P. Mattock June 15, 2010, 3:56 a.m. UTC | #4
On 06/14/2010 08:49 PM, Valdis.Kletnieks@vt.edu wrote:
> On Mon, 14 Jun 2010 19:12:31 PDT, "Justin P. Mattock" said:
>
>> what I tried was this:
>>
>> if (!rc)
>> 	printk("test........"\n")
>>
>> and everything looked good,
>> but as a soon as I changed
>>
>> rc = transmit_cmd(chip,&tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
>>      			"attempting to determine the timeouts");
>>
>> to this:
>>
>> rc = transmit_cmd(chip,&tpm_cmd, TPM_INTERNAL_RESULT_SIZE);
>>
>> if (!rc)
>> 	printk("attempting to determine the timeouts\n");
>
> *baffled* Why did you think that would work? transmit_cmd()s signature
> has 4 parameters.

I have no manual in front of me. Did a quick google, but came up with 
(no hits) info on what that function does. grep showed too many entries 
to really see why/what this is. So I kind of just scrambled with this one.

Justin P. Mattock
--
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
  
Peter Stuge June 15, 2010, 5:29 a.m. UTC | #5
Justin P. Mattock wrote:
> > *baffled* Why did you think that would work? transmit_cmd()s signature
> > has 4 parameters.
> 
> I have no manual in front of me. Did a quick google, but came up with 
> (no hits) info on what that function does. grep showed too many entries 
> to really see why/what this is.

Check out the tool cscope. (Or kscope, if you prefer a GUI.)


//Peter
--
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
  
Justin P. Mattock June 15, 2010, 5:58 a.m. UTC | #6
On 06/14/2010 10:29 PM, Peter Stuge wrote:
> Justin P. Mattock wrote:
>>> *baffled* Why did you think that would work? transmit_cmd()s signature
>>> has 4 parameters.
>>
>> I have no manual in front of me. Did a quick google, but came up with
>> (no hits) info on what that function does. grep showed too many entries
>> to really see why/what this is.
>
> Check out the tool cscope. (Or kscope, if you prefer a GUI.)
>
>
> //Peter
>

thanks for this tool.. I think this is what I need.. running around not 
knowing what/where the manual is for a call is a bit daunting.
I'll give this a look.

Thanks for this..

Justin P. Mattock
--
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
  
Jean Delvare June 15, 2010, 9:19 a.m. UTC | #7
On Tue, 15 Jun 2010 07:29:44 +0200, Peter Stuge wrote:
> Justin P. Mattock wrote:
> > > *baffled* Why did you think that would work? transmit_cmd()s signature
> > > has 4 parameters.
> > 
> > I have no manual in front of me. Did a quick google, but came up with 
> > (no hits) info on what that function does. grep showed too many entries 
> > to really see why/what this is.

Justin, I think you're on the wrong track here. You seem to be in a
hurry to fix all these warnings generated by gcc 4.6. There is no hurry
at all. Most warnings are false positives, and actual bugs may take
some thinking and knowledge. So rushing is not needed and not
desirable. Going too fast, you might even introduce new bugs, or
prevent old bugs from being properly fixed.

Warnings are a chance to make the code better. The goal is not to fix
them quickly, but to fix them properly. If this is not your intent,
then please stop immediately and let others deal with these warnings.
If you want to help, this is appreciated, but what we need it quality,
not quantity.

> Check out the tool cscope. (Or kscope, if you prefer a GUI.)

Or just LXR online if you don't want to install anything:

http://lxr.linux.no/linux
http://lxr.linux.no/#linux+v2.6.34/drivers/char/tpm/tpm.c#L451
  
Justin P. Mattock June 15, 2010, 9:41 a.m. UTC | #8
On 06/15/2010 02:19 AM, Jean Delvare wrote:
> On Tue, 15 Jun 2010 07:29:44 +0200, Peter Stuge wrote:
>> Justin P. Mattock wrote:
>>>> *baffled* Why did you think that would work? transmit_cmd()s signature
>>>> has 4 parameters.
>>>
>>> I have no manual in front of me. Did a quick google, but came up with
>>> (no hits) info on what that function does. grep showed too many entries
>>> to really see why/what this is.
>
> Justin, I think you're on the wrong track here. You seem to be in a
> hurry to fix all these warnings generated by gcc 4.6. There is no hurry
> at all. Most warnings are false positives, and actual bugs may take
> some thinking and knowledge. So rushing is not needed and not
> desirable. Going too fast, you might even introduce new bugs, or
> prevent old bugs from being properly fixed.
>
> Warnings are a chance to make the code better. The goal is not to fix
> them quickly, but to fix them properly. If this is not your intent,
> then please stop immediately and let others deal with these warnings.
> If you want to help, this is appreciated, but what we need it quality,
> not quantity.
>

your right.. I do have this "must get it fixed now, or else
cut your head of mentality".. causing me to rush through things..
who knows why I do this..(I dont know why I do this honestly)

>> Check out the tool cscope. (Or kscope, if you prefer a GUI.)
>
> Or just LXR online if you don't want to install anything:
>
> http://lxr.linux.no/linux
> http://lxr.linux.no/#linux+v2.6.34/drivers/char/tpm/tpm.c#L451
>

At this point though gentlemen/ladies I'm pretty much crapped out now!!
so any answer is going to be skewed. but rushing through things is not 
good.

Justin P. Mattock

--
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
  

Patch

diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index 05ad4a1..3d685dc 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -514,6 +514,8 @@  void tpm_gen_interrupt(struct tpm_chip *chip)
 
 	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
 			"attempting to determine the timeouts");
+	if (!rc)
+		rc = 0;
 }
 EXPORT_SYMBOL_GPL(tpm_gen_interrupt);