[PATCH 0/8] Fix gcc 4.6.0 set but not used warning messages.

[PATCH 0/8] Fix gcc 4.6.0 set but not used warning messages.

[Justin P. Mattock]

First and foremost, I must
thank anybody taking the time to even
look at these(I know you people have better
things to be doing).

And secondly here is my try at trying
to fix some of the warning messages
spammed by gcc 4.6.0 when building the
kernel. Some of them I removed, and
some of them I just shut off.

Note: Removing the code does seem like a
good approach(if it's actually dead),
but if not then something needs
to be fixed.
As for shutting off the code to shutup gcc
does seem like a temporary fix, but would
rather have a warning message, than see it get
lost in the sands of time.

In any case Thanks for taking the time,
and hopefully we can get fixes for all of
this mess generated by gcc..

Justin P. Mattock

[PATCH 1/8]reiserfs:stree.c Fix variable set but not used.

[Justin P. Mattock]

Not sure if this is correct or not.
the below patch gets rid of this warning message
produced by gcc 4.6.0

fs/reiserfs/stree.c: In function 'search_by_key':
fs/reiserfs/stree.c:602:6: warning: variable 'right_neighbor_of_leaf_node' set but not used

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

---
 fs/reiserfs/stree.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/reiserfs/stree.c b/fs/reiserfs/stree.c
index 313d39d..73086ad 100644
--- a/fs/reiserfs/stree.c
+++ b/fs/reiserfs/stree.c
@@ -599,7 +599,6 @@ int search_by_key(struct super_block *sb, const struct cpu_key *key,	/* Key to s
 	struct buffer_head *bh;
 	struct path_element *last_element;
 	int node_level, retval;
-	int right_neighbor_of_leaf_node;
 	int fs_gen;
 	struct buffer_head *reada_bh[SEARCH_BY_KEY_READA];
 	b_blocknr_t reada_blocks[SEARCH_BY_KEY_READA];
@@ -617,8 +616,7 @@ int search_by_key(struct super_block *sb, const struct cpu_key *key,	/* Key to s
 
 	pathrelse(search_path);
 
-	right_neighbor_of_leaf_node = 0;
-
+	
 	/* With each iteration of this loop we search through the items in the
 	   current node, and calculate the next current node(next path element)
 	   for the next iteration of this loop.. */
@@ -695,8 +693,7 @@ int search_by_key(struct super_block *sb, const struct cpu_key *key,	/* Key to s
 			   starting from the root. */
 			block_number = SB_ROOT_BLOCK(sb);
 			expected_level = -1;
-			right_neighbor_of_leaf_node = 0;
-
+			
 			/* repeat search from the root */
 			continue;
 		}
-- 
1.7.1.rc1.21.gf3bd6

Re: [PATCH 1/8]reiserfs:stree.c Fix variable set but not used.

[Nick Bowler]

On 13:26 Mon 14 Jun     , Justin P. Mattock wrote:
> @@ -617,8 +616,7 @@ int search_by_key(struct super_block *sb, const struct cpu_key *key,	/* Key to s
>  
>  	pathrelse(search_path);
>  
> -	right_neighbor_of_leaf_node = 0;
> -
> +	

This hunk introduces whitespace on the empty line, which is not cool.

>  	/* With each iteration of this loop we search through the items in the
>  	   current node, and calculate the next current node(next path element)
>  	   for the next iteration of this loop.. */
> @@ -695,8 +693,7 @@ int search_by_key(struct super_block *sb, const struct cpu_key *key,	/* Key to s
>  			   starting from the root. */
>  			block_number = SB_ROOT_BLOCK(sb);
>  			expected_level = -1;
> -			right_neighbor_of_leaf_node = 0;
> -
> +			

Here, too.

Most of the patches in this series have similar issues.

-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

Re: [PATCH 1/8]reiserfs:stree.c Fix variable set but not used.

[Justin P. Mattock]

On 06/14/2010 01:48 PM, Nick Bowler wrote:
> On 13:26 Mon 14 Jun     , Justin P. Mattock wrote:
>> @@ -617,8 +616,7 @@ int search_by_key(struct super_block *sb, const struct cpu_key *key,	/* Key to s
>>
>>   	pathrelse(search_path);
>>
>> -	right_neighbor_of_leaf_node = 0;
>> -
>> +	
>
> This hunk introduces whitespace on the empty line, which is not cool.

I can resend!!(biggest problem is working
through these warnings)

>
>>   	/* With each iteration of this loop we search through the items in the
>>   	   current node, and calculate the next current node(next path element)
>>   	   for the next iteration of this loop.. */
>> @@ -695,8 +693,7 @@ int search_by_key(struct super_block *sb, const struct cpu_key *key,	/* Key to s
>>   			   starting from the root. */
>>   			block_number = SB_ROOT_BLOCK(sb);
>>   			expected_level = -1;
>> -			right_neighbor_of_leaf_node = 0;
>> -
>> +			
>
> Here, too.
>
> Most of the patches in this series have similar issues.
>

main thing now(for me atleast)is,
is this actual dead code or what?
if not then something else needs to
be done, if yes then I guess I can
resend this, with out the whitespace
issue.

Justin P. Mattock

Re: [PATCH 1/8]reiserfs:stree.c Fix variable set but not used.

[Edward Shishkin]

Justin P. Mattock wrote:
> Not sure if this is correct or not.
> the below patch gets rid of this warning message
> produced by gcc 4.6.0
>
> fs/reiserfs/stree.c: In function 'search_by_key':
> fs/reiserfs/stree.c:602:6: warning: variable 'right_neighbor_of_leaf_node' set but not used
>
>  Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
>   

Acked-by: Edward Shishkin <edward.shishkin@gmail.com>

> ---
>  fs/reiserfs/stree.c |    7 ++-----
>  1 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/fs/reiserfs/stree.c b/fs/reiserfs/stree.c
> index 313d39d..73086ad 100644
> --- a/fs/reiserfs/stree.c
> +++ b/fs/reiserfs/stree.c
> @@ -599,7 +599,6 @@ int search_by_key(struct super_block *sb, const struct cpu_key *key,	/* Key to s
>  	struct buffer_head *bh;
>  	struct path_element *last_element;
>  	int node_level, retval;
> -	int right_neighbor_of_leaf_node;
>  	int fs_gen;
>  	struct buffer_head *reada_bh[SEARCH_BY_KEY_READA];
>  	b_blocknr_t reada_blocks[SEARCH_BY_KEY_READA];
> @@ -617,8 +616,7 @@ int search_by_key(struct super_block *sb, const struct cpu_key *key,	/* Key to s
>  
>  	pathrelse(search_path);
>  
> -	right_neighbor_of_leaf_node = 0;
> -
> +	
>  	/* With each iteration of this loop we search through the items in the
>  	   current node, and calculate the next current node(next path element)
>  	   for the next iteration of this loop.. */
> @@ -695,8 +693,7 @@ int search_by_key(struct super_block *sb, const struct cpu_key *key,	/* Key to s
>  			   starting from the root. */
>  			block_number = SB_ROOT_BLOCK(sb);
>  			expected_level = -1;
> -			right_neighbor_of_leaf_node = 0;
> -
> +			
>  			/* repeat search from the root */
>  			continue;
>  		}
>   

Re: [PATCH 1/8]reiserfs:stree.c Fix variable set but not used.

[Justin P. Mattock]

On 06/14/2010 02:05 PM, Edward Shishkin wrote:
> Justin P. Mattock wrote:
>> Not sure if this is correct or not.
>> the below patch gets rid of this warning message
>> produced by gcc 4.6.0
>>
>> fs/reiserfs/stree.c: In function 'search_by_key':
>> fs/reiserfs/stree.c:602:6: warning: variable
>> 'right_neighbor_of_leaf_node' set but not used
>>
>> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
>
> Acked-by: Edward Shishkin <edward.shishkin@gmail.com>
>

o.k.!!
what about the whitespace issue?
from what I remember I did notice the "+"
that git does when making patches like this
but given some many of these warnings I just
did a quick workaround or however then figured
to worry later on that.

>> ---
>> fs/reiserfs/stree.c | 7 ++-----
>> 1 files changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/reiserfs/stree.c b/fs/reiserfs/stree.c
>> index 313d39d..73086ad 100644
>> --- a/fs/reiserfs/stree.c
>> +++ b/fs/reiserfs/stree.c
>> @@ -599,7 +599,6 @@ int search_by_key(struct super_block *sb, const
>> struct cpu_key *key, /* Key to s
>> struct buffer_head *bh;
>> struct path_element *last_element;
>> int node_level, retval;
>> - int right_neighbor_of_leaf_node;
>> int fs_gen;
>> struct buffer_head *reada_bh[SEARCH_BY_KEY_READA];
>> b_blocknr_t reada_blocks[SEARCH_BY_KEY_READA];
>> @@ -617,8 +616,7 @@ int search_by_key(struct super_block *sb, const
>> struct cpu_key *key, /* Key to s
>>
>> pathrelse(search_path);
>>
>> - right_neighbor_of_leaf_node = 0;
>> -
>> +
>> /* With each iteration of this loop we search through the items in the
>> current node, and calculate the next current node(next path element)
>> for the next iteration of this loop.. */
>> @@ -695,8 +693,7 @@ int search_by_key(struct super_block *sb, const
>> struct cpu_key *key, /* Key to s
>> starting from the root. */
>> block_number = SB_ROOT_BLOCK(sb);
>> expected_level = -1;
>> - right_neighbor_of_leaf_node = 0;
>> -
>> +
>> /* repeat search from the root */
>> continue;
>> }
>
>

Re: [PATCH 1/8]reiserfs:stree.c Fix variable set but not used.

[Edward Shishkin]

Justin P. Mattock wrote:
> On 06/14/2010 02:05 PM, Edward Shishkin wrote:
>> Justin P. Mattock wrote:
>>> Not sure if this is correct or not.
>>> the below patch gets rid of this warning message
>>> produced by gcc 4.6.0
>>>
>>> fs/reiserfs/stree.c: In function 'search_by_key':
>>> fs/reiserfs/stree.c:602:6: warning: variable
>>> 'right_neighbor_of_leaf_node' set but not used
>>>
>>> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
>>
>> Acked-by: Edward Shishkin <edward.shishkin@gmail.com>
>>
>
> o.k.!!
> what about the whitespace issue?

Whitespaces should be removed.
I recommend quilt package for managing patches:
"quilt refresh --strip-trailing-whitespace" is your friend..

Thanks,
Edward.

>
> from what I remember I did notice the "+"
> that git does when making patches like this
> but given some many of these warnings I just
> did a quick workaround or however then figured
> to worry later on that.
>
>>> ---
>>> fs/reiserfs/stree.c | 7 ++-----
>>> 1 files changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/reiserfs/stree.c b/fs/reiserfs/stree.c
>>> index 313d39d..73086ad 100644
>>> --- a/fs/reiserfs/stree.c
>>> +++ b/fs/reiserfs/stree.c
>>> @@ -599,7 +599,6 @@ int search_by_key(struct super_block *sb, const
>>> struct cpu_key *key, /* Key to s
>>> struct buffer_head *bh;
>>> struct path_element *last_element;
>>> int node_level, retval;
>>> - int right_neighbor_of_leaf_node;
>>> int fs_gen;
>>> struct buffer_head *reada_bh[SEARCH_BY_KEY_READA];
>>> b_blocknr_t reada_blocks[SEARCH_BY_KEY_READA];
>>> @@ -617,8 +616,7 @@ int search_by_key(struct super_block *sb, const
>>> struct cpu_key *key, /* Key to s
>>>
>>> pathrelse(search_path);
>>>
>>> - right_neighbor_of_leaf_node = 0;
>>> -
>>> +
>>> /* With each iteration of this loop we search through the items in the
>>> current node, and calculate the next current node(next path element)
>>> for the next iteration of this loop.. */
>>> @@ -695,8 +693,7 @@ int search_by_key(struct super_block *sb, const
>>> struct cpu_key *key, /* Key to s
>>> starting from the root. */
>>> block_number = SB_ROOT_BLOCK(sb);
>>> expected_level = -1;
>>> - right_neighbor_of_leaf_node = 0;
>>> -
>>> +
>>> /* repeat search from the root */
>>> continue;
>>> }
>>
>>
>
>

Re: [PATCH 1/8]reiserfs:stree.c Fix variable set but not used.

[Justin P. Mattock]

On 06/14/2010 02:47 PM, Edward Shishkin wrote:
> Justin P. Mattock wrote:
>> On 06/14/2010 02:05 PM, Edward Shishkin wrote:
>>> Justin P. Mattock wrote:
>>>> Not sure if this is correct or not.
>>>> the below patch gets rid of this warning message
>>>> produced by gcc 4.6.0
>>>>
>>>> fs/reiserfs/stree.c: In function 'search_by_key':
>>>> fs/reiserfs/stree.c:602:6: warning: variable
>>>> 'right_neighbor_of_leaf_node' set but not used
>>>>
>>>> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
>>>
>>> Acked-by: Edward Shishkin <edward.shishkin@gmail.com>
>>>
>>
>> o.k.!!
>> what about the whitespace issue?
>
> Whitespaces should be removed.
> I recommend quilt package for managing patches:
> "quilt refresh --strip-trailing-whitespace" is your friend..
>
> Thanks,
> Edward.
>

o.k. I resent this.. fixed the whitespace(hopefully)
and add your Acked to it.
as for quilt I'll have to look into that..
(using a lfs system, so if the sourcecode is easy
to deal with(build), then it's a good but if it becomes
a nightmare maybe not!!).


Justin P. Mattock

Re: [PATCH 1/8]reiserfs:stree.c Fix variable set but not used.

[Stefan Richter]

On 14 Jun, Justin P. Mattock wrote:
> On 06/14/2010 02:47 PM, Edward Shishkin wrote:
>> Whitespaces should be removed.
>> I recommend quilt package for managing patches:
>> "quilt refresh --strip-trailing-whitespace" is your friend..
> 
> o.k. I resent this.. fixed the whitespace(hopefully)
> and add your Acked to it.
> as for quilt I'll have to look into that..
> (using a lfs system, so if the sourcecode is easy
> to deal with(build), then it's a good but if it becomes
> a nightmare maybe not!!).

Since you appear to generate the patches with git, you can use "git diff
--check [...]" for some basic whitespace checks (additions of trailing
space, additions of space before tab).  For more extensive checks, try
"git diff [...] | scripts/checkpatch.pl -".  Check this before you
commit.  If you committed already, "git commit --amend [-a] [...]" lets
you alter the very last commit of course.
-- 
Stefan Richter
-=====-==-=- -==- -====
http://arcgraph.de/sr/

Re: [PATCH 1/8]reiserfs:stree.c Fix variable set but not used.

[Justin P. Mattock]

On 06/14/2010 04:07 PM, Stefan Richter wrote:
> On 14 Jun, Justin P. Mattock wrote:
>> On 06/14/2010 02:47 PM, Edward Shishkin wrote:
>>> Whitespaces should be removed.
>>> I recommend quilt package for managing patches:
>>> "quilt refresh --strip-trailing-whitespace" is your friend..
>>
>> o.k. I resent this.. fixed the whitespace(hopefully)
>> and add your Acked to it.
>> as for quilt I'll have to look into that..
>> (using a lfs system, so if the sourcecode is easy
>> to deal with(build), then it's a good but if it becomes
>> a nightmare maybe not!!).
>
> Since you appear to generate the patches with git, you can use "git diff
> --check [...]" for some basic whitespace checks (additions of trailing
> space, additions of space before tab).  For more extensive checks, try
> "git diff [...] | scripts/checkpatch.pl -".  Check this before you
> commit.  If you committed already, "git commit --amend [-a] [...]" lets
> you alter the very last commit of course.


Thanks for the info on this, copied it
down in my book of commands...

Justin P. Mattock

[PATCH 2/8]bluetooth/hci_ldisc.c Fix warning: variable 'tty' set but not used

[Justin P. Mattock]

Im getting this while building:
  CC [M]  drivers/bluetooth/hci_ldisc.o
drivers/bluetooth/hci_ldisc.c: In function 'hci_uart_send_frame':
drivers/bluetooth/hci_ldisc.c:213:21: warning: variable 'tty' set but not used

the below fixed it for me, but am not sure if
it's correct.

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

---
 drivers/bluetooth/hci_ldisc.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 76a1abb..f693dfe 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -210,7 +210,6 @@ static int hci_uart_close(struct hci_dev *hdev)
 static int hci_uart_send_frame(struct sk_buff *skb)
 {
 	struct hci_dev* hdev = (struct hci_dev *) skb->dev;
-	struct tty_struct *tty;
 	struct hci_uart *hu;
 
 	if (!hdev) {
@@ -222,8 +221,7 @@ static int hci_uart_send_frame(struct sk_buff *skb)
 		return -EBUSY;
 
 	hu = (struct hci_uart *) hdev->driver_data;
-	tty = hu->tty;
-
+	
 	BT_DBG("%s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type, skb->len);
 
 	hu->proto->enqueue(hu, skb);
-- 
1.7.1.rc1.21.gf3bd6

Re: [PATCH 2/8]bluetooth/hci_ldisc.c Fix warning: variable 'tty' set but not used

[Gustavo F. Padovan]

Hi Justin,

* Justin P. Mattock <justinmattock@gmail.com> [2010-06-14 13:26:42 -0700]:

> Im getting this while building:
>   CC [M]  drivers/bluetooth/hci_ldisc.o
> drivers/bluetooth/hci_ldisc.c: In function 'hci_uart_send_frame':
> drivers/bluetooth/hci_ldisc.c:213:21: warning: variable 'tty' set but not used
> 
> the below fixed it for me, but am not sure if
> it's correct.


The fix is correct, you just need to fix the trailing whitespace
problem and resend it.
Also we use "Bluetooth:" as part of the commit message on the bluetooth
subsystem. For example:

"Bluetooth: Remove set but not used varible 'tty' 

Or something like that.

-- 
Gustavo F. Padovan
http://padovan.org

[PATCH 3/8]char/hpet.c Fix variable 'hpet' set but not used

[Justin P. Mattock]

The below fixes this warning:
drivers/char/hpet.c: In function 'hpet_ioctl_common':
drivers/char/hpet.c:559:23: warning: variable 'hpet' set but not used

please have a look.
 Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

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

diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index a0a1829..7932858 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -556,7 +556,6 @@ static int
 hpet_ioctl_common(struct hpet_dev *devp, int cmd, unsigned long arg, int kernel)
 {
 	struct hpet_timer __iomem *timer;
-	struct hpet __iomem *hpet;
 	struct hpets *hpetp;
 	int err;
 	unsigned long v;
@@ -568,7 +567,6 @@ hpet_ioctl_common(struct hpet_dev *devp, int cmd, unsigned long arg, int kernel)
 	case HPET_DPI:
 	case HPET_IRQFREQ:
 		timer = devp->hd_timer;
-		hpet = devp->hd_hpet;
 		hpetp = devp->hd_hpets;
 		break;
 	case HPET_IE_ON:
-- 
1.7.1.rc1.21.gf3bd6

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

[Justin P. Mattock]

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

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);
 
-- 
1.7.1.rc1.21.gf3bd6

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

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

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?

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

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

[Justin P. Mattock]

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

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

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

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.

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

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

[Justin P. Mattock]

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

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

[Peter Stuge]

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

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

[Justin P. Mattock]

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

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

[Stefan Richter]

Justin P. Mattock wrote:
> On 06/14/2010 10:29 PM, Peter Stuge wrote:
>> Justin P. Mattock wrote:
>>> 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.)
> 
> 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.

Similar to cscope/kscope but online:
http://lxr.free-electrons.com/
http://lxr.linux.no/linux

E.g. http://lxr.free-electrons.com/ident?i=transmit_cmd
-- 
Stefan Richter
-=====-==-=- -==- -====
http://arcgraph.de/sr/

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

[Justin P. Mattock]

On 06/14/2010 11:57 PM, Stefan Richter wrote:
> Justin P. Mattock wrote:
>> On 06/14/2010 10:29 PM, Peter Stuge wrote:
>>> Justin P. Mattock wrote:
>>>> 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.)
>>
>> 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.
>
> Similar to cscope/kscope but online:
> http://lxr.free-electrons.com/
> http://lxr.linux.no/linux
>
> E.g. http://lxr.free-electrons.com/ident?i=transmit_cmd

hmm.. yeah I use those guys a lot.. with grep also.. I think what might 
throw me off is things like def.orig = def.new then def.orig gets lost 
in the mix(or it's there, but the grep becomes gigantic in code 
results.) main thing I look for is googling "somedefinition linux man"
for some idea of what that function does.

Justin P. Mattock

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

[Stefan Richter]

(adding Cc: tpm maintainers, after I had cut the Cc list earlier...)

Justin P. Mattock wrote:
> On 06/14/2010 11:57 PM, Stefan Richter wrote:
>> Justin P. Mattock wrote:
>>> On 06/14/2010 10:29 PM, Peter Stuge wrote:
>>>> Justin P. Mattock wrote:
>>>>> 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.)
>>>
>>> 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.
>>
>> Similar to cscope/kscope but online:
>> http://lxr.free-electrons.com/
>> http://lxr.linux.no/linux
>>
>> E.g. http://lxr.free-electrons.com/ident?i=transmit_cmd
> 
> hmm.. yeah I use those guys a lot.. with grep also.. I think what might
> throw me off is things like def.orig = def.new then def.orig gets lost
> in the mix(or it's there, but the grep becomes gigantic in code
> results.) main thing I look for is googling "somedefinition linux man"
> for some idea of what that function does.
> 
> Justin P. Mattock

I'm afraid that even if there was inline documentation of the function,
it would still be near impossible to guess what to do with unused return
codes if one is not familiar with the hardware and with the driver.

To quote your original patch again:

> 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(-)
> 
> 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);

So, void tpm_gen_interrupt() itself clearly is not prepared to do
anything about an error return from transmit_cmd.  The question is, is
this OK and can the "rc = " simply be removed, or should
tpm_gen_interrupt gain some error handling, or should tpm_gen_interrupt
be changed to a non-void function and its callers check for errors in
tpm_gen_interrupt?

It could very well be that there is really no need or no possibility for
error handling.  But until somebody familiar with the matter has figured
this out, it is in fact better to leave the compiler warning in place.

Just silencing the compiler if the possibility exists that there is an
actual flaw here is not the way to go.  Still, good that you reported it
even with an RFC patch, since the authors were obviously unaware of such
a warning until now.
-- 
Stefan Richter
-=====-==-=- -==- -====
http://arcgraph.de/sr/

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

[Justin P. Mattock]

On 06/15/2010 12:56 AM, Stefan Richter wrote:
> (adding Cc: tpm maintainers, after I had cut the Cc list earlier...)
>
> Justin P. Mattock wrote:
>> On 06/14/2010 11:57 PM, Stefan Richter wrote:
>>> Justin P. Mattock wrote:
>>>> On 06/14/2010 10:29 PM, Peter Stuge wrote:
>>>>> Justin P. Mattock wrote:
>>>>>> 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.)
>>>>
>>>> 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.
>>>
>>> Similar to cscope/kscope but online:
>>> http://lxr.free-electrons.com/
>>> http://lxr.linux.no/linux
>>>
>>> E.g. http://lxr.free-electrons.com/ident?i=transmit_cmd
>>
>> hmm.. yeah I use those guys a lot.. with grep also.. I think what might
>> throw me off is things like def.orig = def.new then def.orig gets lost
>> in the mix(or it's there, but the grep becomes gigantic in code
>> results.) main thing I look for is googling "somedefinition linux man"
>> for some idea of what that function does.
>>
>> Justin P. Mattock
>
> I'm afraid that even if there was inline documentation of the function,
> it would still be near impossible to guess what to do with unused return
> codes if one is not familiar with the hardware and with the driver.
>
> To quote your original patch again:
>
>> 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(-)
>>
>> 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);
>
> So, void tpm_gen_interrupt() itself clearly is not prepared to do
> anything about an error return from transmit_cmd.  The question is, is
> this OK and can the "rc = " simply be removed, or should
> tpm_gen_interrupt gain some error handling, or should tpm_gen_interrupt
> be changed to a non-void function and its callers check for errors in
> tpm_gen_interrupt?
>

still not sure what that mechanism is doing..

I was going by the warning message i.g.  set but not used
so as soon as it was being used even by adding printk, gcc was happy.
But shutting it off seemed a better approach why? because at the time I 
knew it would work, or fixed it temporarily until somebody who's code 
this is can look at it and send out a better solution(hence what you did).

> It could very well be that there is really no need or no possibility for
> error handling.  But until somebody familiar with the matter has figured
> this out, it is in fact better to leave the compiler warning in place.
>

yep.. I agree..

> Just silencing the compiler if the possibility exists that there is an
> actual flaw here is not the way to go.  Still, good that you reported it
> even with an RFC patch, since the authors were obviously unaware of such
> a warning until now.

I did not like the idea of silencing it.. due to the fact that it can be 
forgotten(or lost in the sands of time per-see)
best I just file a bug report then..in these types of cases.

In any case I still have more warnings..(and that's just my .config)
who knows how many more there are.. If you guys like I can go ahead in 
the next few days and send out another set(minus the rc = 0; thing that 
I did) then go from there.

keep in mind I'm just tackling the ones I think are easy.. ones with 
double variables I left until later down the line..

Justin P. Mattock

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

[Jean Delvare]

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

-- 
Jean Delvare

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

[Justin P. Mattock]

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

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

[Sergey V.]

On Tuesday 15 of June 2010 00:26:44 Justin P. Mattock wrote:
> 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(-)
> 
> 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);
>  
> -- 
> 1.7.1.rc1.21.gf3bd6
> 

Hi Justin

IMHO
See code of functions tpm_transmit(), transmit_cmd and tpm_gen_interrupt(). 
In tpm_gen_interrupt() not need check rc for wrong value bacause if in function 
transmit_cmd() len == TPM_ERROR_SIZE then put a debug message (dev_dbg()).
Again, if something wrong in tpm_transmit() then runs dev_err() and rc in 
tpm_gen_interrupt() get -E* value.
So, we can remove unused rc variable in tpm_gen_interrupt(). 

See patch below. Note: I not tested it.


Subject: [PATCH] drivers: tpm.c: Remove unused variable 'rc'

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

diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index 05ad4a1..f9f5b47 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -505,15 +505,14 @@ ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, 
cap_t *cap,
 void tpm_gen_interrupt(struct tpm_chip *chip)
 {
 	struct	tpm_cmd_t tpm_cmd;
-	ssize_t rc;
 
 	tpm_cmd.header.in = tpm_getcap_header;
 	tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
 	tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
 	tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
 
-	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
-			"attempting to determine the timeouts");
+	transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
+		     "attempting to determine the timeouts");
 }
 EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
 
-- 
1.7.1

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

[Justin P. Mattock]

On 06/15/2010 11:53 AM, Sergey V. wrote:
> On Tuesday 15 of June 2010 00:26:44 Justin P. Mattock wrote:
>> 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(-)
>>
>> 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);
>>
>> --
>> 1.7.1.rc1.21.gf3bd6
>>
>
> Hi Justin
>
> IMHO
> See code of functions tpm_transmit(), transmit_cmd and tpm_gen_interrupt().
> In tpm_gen_interrupt() not need check rc for wrong value bacause if in function
> transmit_cmd() len == TPM_ERROR_SIZE then put a debug message (dev_dbg()).
> Again, if something wrong in tpm_transmit() then runs dev_err() and rc in
> tpm_gen_interrupt() get -E* value.
> So, we can remove unused rc variable in tpm_gen_interrupt().
>
> See patch below. Note: I not tested it.
>
>
> Subject: [PATCH] drivers: tpm.c: Remove unused variable 'rc'
>
> ---
>   drivers/char/tpm/tpm.c |    5 ++---
>   1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> index 05ad4a1..f9f5b47 100644
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -505,15 +505,14 @@ ssize_t tpm_getcap(struct device *dev, __be32 subcap_id,
> cap_t *cap,
>   void tpm_gen_interrupt(struct tpm_chip *chip)
>   {
>   	struct	tpm_cmd_t tpm_cmd;
> -	ssize_t rc;
>
>   	tpm_cmd.header.in = tpm_getcap_header;
>   	tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
>   	tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
>   	tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
>
> -	rc = transmit_cmd(chip,&tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> -			"attempting to determine the timeouts");
> +	transmit_cmd(chip,&tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> +		     "attempting to determine the timeouts");
>   }
>   EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
>



o.k. applied this patch and rebuilt, here is what I see:

   CC [M]  drivers/char/ipmi/ipmi_poweroff.o
   CC      drivers/char/tpm/tpm.o
   CC      drivers/char/tpm/tpm_bios.o
   CC      drivers/char/tpm/tpm_tis.o
   LD      drivers/char/tpm/built-in.o


looks good over here Thanks for sending this..

Justin P. Mattock

[PATCH 5/8]drm:drm_gem Fix warning: variable 'dev' set but not used

[Justin P. Mattock]

Probably not even a fix for this warning:

  CC [M]  drivers/gpu/drm/drm_gem.o
drivers/gpu/drm/drm_gem.c: In function 'drm_gem_handle_delete':
drivers/gpu/drm/drm_gem.c:188:21: warning: variable 'dev' set but not used

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

---
 drivers/gpu/drm/drm_gem.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 33dad3f..e8180c9 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -206,6 +206,8 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
 		return -EINVAL;
 	}
 	dev = obj->dev;
+	if (!dev)
+		dev = 0;
 
 	/* Release reference and decrement refcount. */
 	idr_remove(&filp->object_idr, handle);
-- 
1.7.1.rc1.21.gf3bd6

[PATCH 6/8]i2c:i2c_core Fix warning: variable 'dummy' set but not used

[Justin P. Mattock]

could be a right solution, could be wrong
here is the warning:
  CC      drivers/i2c/i2c-core.o
drivers/i2c/i2c-core.c: In function 'i2c_register_adapter':
drivers/i2c/i2c-core.c:757:15: warning: variable 'dummy' set but not used
 
 Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

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

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 1cca263..79c6c26 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -794,6 +794,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 	mutex_lock(&core_lock);
 	dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap,
 				 __process_new_adapter);
+	if(!dummy)
+		dummy = 0;
 	mutex_unlock(&core_lock);
 
 	return 0;
-- 
1.7.1.rc1.21.gf3bd6

Re: [PATCH 6/8]i2c:i2c_core Fix warning: variable 'dummy' set but not used

[Jean Delvare]

Hi Justin,

On Mon, 14 Jun 2010 13:26:46 -0700, Justin P. Mattock wrote:
> could be a right solution, could be wrong
> here is the warning:
>   CC      drivers/i2c/i2c-core.o
> drivers/i2c/i2c-core.c: In function 'i2c_register_adapter':
> drivers/i2c/i2c-core.c:757:15: warning: variable 'dummy' set but not used
>  
>  Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
> 
> ---
>  drivers/i2c/i2c-core.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 1cca263..79c6c26 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -794,6 +794,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>  	mutex_lock(&core_lock);
>  	dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap,
>  				 __process_new_adapter);
> +	if(!dummy)
> +		dummy = 0;

One word: scripts/checkpatch.pl

In other news, the above is just plain wrong. First we force people to
read the result of bus_for_each_drv() and then when they do and don't
need the value, gcc complains, so we add one more layer of useless
code, which developers and possibly tools will later wonder and
complain about? I can easily imagine that a static code analyzer would
spot the above code as being a potential bug.

Let's stop this madness now please.

Either __must_check goes away from bus_for_each_drv() and from every
other function which raises this problem, or we must disable that new
type of warning gcc 4.6.0 generates. Depends which warnings we value
more, as we can't sanely have both.

>  	mutex_unlock(&core_lock);
>  
>  	return 0;


-- 
Jean Delvare

Re: [PATCH 6/8]i2c:i2c_core Fix warning: variable 'dummy' set but not used

[Justin P. Mattock]

On 06/14/2010 01:53 PM, Jean Delvare wrote:
> Hi Justin,
>
> On Mon, 14 Jun 2010 13:26:46 -0700, Justin P. Mattock wrote:
>> could be a right solution, could be wrong
>> here is the warning:
>>    CC      drivers/i2c/i2c-core.o
>> drivers/i2c/i2c-core.c: In function 'i2c_register_adapter':
>> drivers/i2c/i2c-core.c:757:15: warning: variable 'dummy' set but not used
>>
>>   Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
>>
>> ---
>>   drivers/i2c/i2c-core.c |    2 ++
>>   1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>> index 1cca263..79c6c26 100644
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
>> @@ -794,6 +794,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>>   	mutex_lock(&core_lock);
>>   	dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap,
>>   				 __process_new_adapter);
>> +	if(!dummy)
>> +		dummy = 0;
>
> One word: scripts/checkpatch.pl

it was this, and/or just take the code out
(since I'm a newbie)

>
> In other news, the above is just plain wrong. First we force people to
> read the result of bus_for_each_drv() and then when they do and don't
> need the value, gcc complains, so we add one more layer of useless
> code, which developers and possibly tools will later wonder and
> complain about? I can easily imagine that a static code analyzer would
> spot the above code as being a potential bug.
>
> Let's stop this madness now please.
>

your telling me!! I haven't even compiled all the way
through the kernel yet.(lots of warnings)

> Either __must_check goes away from bus_for_each_drv() and from every
> other function which raises this problem, or we must disable that new
> type of warning gcc 4.6.0 generates. Depends which warnings we value
> more, as we can't sanely have both.
>
>>   	mutex_unlock(&core_lock);
>>
>>   	return 0;
>
>

up to you guys..
best thing now is deciphering what
and what not is an actual issue.

Justin P. Mattock

Re: [PATCH 6/8]i2c:i2c_core Fix warning: variable 'dummy' set but not used

[Jean Delvare]

Hi Justin,

On Mon, 14 Jun 2010 14:06:12 -0700, Justin P. Mattock wrote:
> On 06/14/2010 01:53 PM, Jean Delvare wrote:
> > Hi Justin,
> >
> > On Mon, 14 Jun 2010 13:26:46 -0700, Justin P. Mattock wrote:
> >> could be a right solution, could be wrong
> >> here is the warning:
> >>    CC      drivers/i2c/i2c-core.o
> >> drivers/i2c/i2c-core.c: In function 'i2c_register_adapter':
> >> drivers/i2c/i2c-core.c:757:15: warning: variable 'dummy' set but not used
> >>
> >>   Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
> >>
> >> ---
> >>   drivers/i2c/i2c-core.c |    2 ++
> >>   1 files changed, 2 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> >> index 1cca263..79c6c26 100644
> >> --- a/drivers/i2c/i2c-core.c
> >> +++ b/drivers/i2c/i2c-core.c
> >> @@ -794,6 +794,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
> >>   	mutex_lock(&core_lock);
> >>   	dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap,
> >>   				 __process_new_adapter);
> >> +	if(!dummy)
> >> +		dummy = 0;
> >
> > One word: scripts/checkpatch.pl
> 
> it was this, and/or just take the code out
> (since I'm a newbie)

I was not (yet) arguing on the code itself, but on its format. Any
patch you send should pass the formatting tests performed by
scripts/checkpatch.pl. Thanks.

-- 
Jean Delvare

Re: [PATCH 6/8]i2c:i2c_core Fix warning: variable 'dummy' set but not used

[Justin P. Mattock]

On 06/15/2010 04:43 AM, Jean Delvare wrote:
> Hi Justin,
>
> On Mon, 14 Jun 2010 14:06:12 -0700, Justin P. Mattock wrote:
>> On 06/14/2010 01:53 PM, Jean Delvare wrote:
>>> Hi Justin,
>>>
>>> On Mon, 14 Jun 2010 13:26:46 -0700, Justin P. Mattock wrote:
>>>> could be a right solution, could be wrong
>>>> here is the warning:
>>>>     CC      drivers/i2c/i2c-core.o
>>>> drivers/i2c/i2c-core.c: In function 'i2c_register_adapter':
>>>> drivers/i2c/i2c-core.c:757:15: warning: variable 'dummy' set but not used
>>>>
>>>>    Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
>>>>
>>>> ---
>>>>    drivers/i2c/i2c-core.c |    2 ++
>>>>    1 files changed, 2 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>>>> index 1cca263..79c6c26 100644
>>>> --- a/drivers/i2c/i2c-core.c
>>>> +++ b/drivers/i2c/i2c-core.c
>>>> @@ -794,6 +794,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>>>>    	mutex_lock(&core_lock);
>>>>    	dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap,
>>>>    				 __process_new_adapter);
>>>> +	if(!dummy)
>>>> +		dummy = 0;
>>>
>>> One word: scripts/checkpatch.pl
>>
>> it was this, and/or just take the code out
>> (since I'm a newbie)
>
> I was not (yet) arguing on the code itself, but on its format. Any
> patch you send should pass the formatting tests performed by
> scripts/checkpatch.pl. Thanks.
>


o.k.  I'll make sure I run everything through checkpatch.pl before 
sending anything.

Justin P. Mattock

Re: [PATCH 6/8]i2c:i2c_core Fix warning: variable 'dummy' set but not used

[David Daney]

On 06/14/2010 01:53 PM, Jean Delvare wrote:
> Hi Justin,
>
> On Mon, 14 Jun 2010 13:26:46 -0700, Justin P. Mattock wrote:
>> could be a right solution, could be wrong
>> here is the warning:
>>    CC      drivers/i2c/i2c-core.o
>> drivers/i2c/i2c-core.c: In function 'i2c_register_adapter':
>> drivers/i2c/i2c-core.c:757:15: warning: variable 'dummy' set but not used
>>
>>   Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
>>
>> ---
>>   drivers/i2c/i2c-core.c |    2 ++
>>   1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>> index 1cca263..79c6c26 100644
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
>> @@ -794,6 +794,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>>   	mutex_lock(&core_lock);
>>   	dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap,
>>   				 __process_new_adapter);
>> +	if(!dummy)
>> +		dummy = 0;
>
> One word: scripts/checkpatch.pl
>
> In other news, the above is just plain wrong. First we force people to
> read the result of bus_for_each_drv() and then when they do and don't
> need the value, gcc complains, so we add one more layer of useless
> code, which developers and possibly tools will later wonder and
> complain about? I can easily imagine that a static code analyzer would
> spot the above code as being a potential bug.
>
> Let's stop this madness now please.
>
> Either __must_check goes away from bus_for_each_drv() and from every
> other function which raises this problem, or we must disable that new
> type of warning gcc 4.6.0 generates. Depends which warnings we value
> more, as we can't sanely have both.
>

That is the crux of the whole thing.  Putting in crap to get rid of the 
__must_check warning someone obviously wanted to provoke is just plain 
wrong.

I don't know what the answer is, but in addition to your suggestion of 
removing the __must_check, you might try:

BUG_ON(dummy != WHAT_IT_SHOULD_BE);

or

if (dummy != WHAT_IT_SHOULD_BE)
	panic("nice message here);


or

static inline void i_really_know_what_i_am_doing(int arg)
{
	/*
	 * Trick the compiler because we don't want to
	 * handle error conditions.
	 */
	return;
}

.
.
.

	i_really_know_what_i_am_doing(dummy);



David Daney

Re: [PATCH 6/8]i2c:i2c_core Fix warning: variable 'dummy' set but not used

[Jean Delvare]

Hi David,

On Mon, 14 Jun 2010 14:28:57 -0700, David Daney wrote:
> On 06/14/2010 01:53 PM, Jean Delvare wrote:
> > Hi Justin,
> >
> > On Mon, 14 Jun 2010 13:26:46 -0700, Justin P. Mattock wrote:
> >> could be a right solution, could be wrong
> >> here is the warning:
> >>    CC      drivers/i2c/i2c-core.o
> >> drivers/i2c/i2c-core.c: In function 'i2c_register_adapter':
> >> drivers/i2c/i2c-core.c:757:15: warning: variable 'dummy' set but not used
> >>
> >>   Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
> >>
> >> ---
> >>   drivers/i2c/i2c-core.c |    2 ++
> >>   1 files changed, 2 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> >> index 1cca263..79c6c26 100644
> >> --- a/drivers/i2c/i2c-core.c
> >> +++ b/drivers/i2c/i2c-core.c
> >> @@ -794,6 +794,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
> >>   	mutex_lock(&core_lock);
> >>   	dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap,
> >>   				 __process_new_adapter);
> >> +	if(!dummy)
> >> +		dummy = 0;
> >
> > One word: scripts/checkpatch.pl
> >
> > In other news, the above is just plain wrong. First we force people to
> > read the result of bus_for_each_drv() and then when they do and don't
> > need the value, gcc complains, so we add one more layer of useless
> > code, which developers and possibly tools will later wonder and
> > complain about? I can easily imagine that a static code analyzer would
> > spot the above code as being a potential bug.
> >
> > Let's stop this madness now please.
> >
> > Either __must_check goes away from bus_for_each_drv() and from every
> > other function which raises this problem, or we must disable that new
> > type of warning gcc 4.6.0 generates. Depends which warnings we value
> > more, as we can't sanely have both.
> >
> 
> That is the crux of the whole thing.  Putting in crap to get rid of the 
> __must_check warning someone obviously wanted to provoke is just plain 
> wrong.

__process_new_adapter() calls i2c_do_add_adapter() which always returns
0. Why should I check the return value of bus_for_each_drv() when I
know it will always be 0 by construction?

Also note that the same function is also called through
bus_for_each_dev() somewhere else in i2c-core, and there is no warning
there because bus_for_each_dev() is not marked __must_check. How
consistent is this? If bus_for_each_dev() is OK without __must_check,
then I can't see why bus_for_each_drv() wouldn't be.

> I don't know what the answer is, but in addition to your suggestion of 
> removing the __must_check, you might try:
> 
> BUG_ON(dummy != WHAT_IT_SHOULD_BE);
> 
> or
> 
> if (dummy != WHAT_IT_SHOULD_BE)
> 	panic("nice message here);

Which will never trigger.

> or
> 
> static inline void i_really_know_what_i_am_doing(int arg)
> {
> 	/*
> 	 * Trick the compiler because we don't want to
> 	 * handle error conditions.
> 	 */
> 	return;
> }
> 
> ..
> ..
> ..
> 
> 	i_really_know_what_i_am_doing(dummy);

Which is adding a lot of lines, and might eventually fail when the
compiler becomes smarter (if it isn't already). Thanks but no thanks.
If I really have to chose one of these evils, I'll go for BUG_ON(), at
least the intent is clear and the bloat is minimum.

-- 
Jean Delvare

Re: [PATCH 6/8]i2c:i2c_core Fix warning: variable 'dummy' set but not used

[David Daney]

On 06/15/2010 04:40 AM, Jean Delvare wrote:
> Hi David,
>
> On Mon, 14 Jun 2010 14:28:57 -0700, David Daney wrote:
>> On 06/14/2010 01:53 PM, Jean Delvare wrote:
>>> Hi Justin,
>>>
>>> On Mon, 14 Jun 2010 13:26:46 -0700, Justin P. Mattock wrote:
>>>> could be a right solution, could be wrong
>>>> here is the warning:
>>>>     CC      drivers/i2c/i2c-core.o
>>>> drivers/i2c/i2c-core.c: In function 'i2c_register_adapter':
>>>> drivers/i2c/i2c-core.c:757:15: warning: variable 'dummy' set but not used
>>>>
>>>>    Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
>>>>
>>>> ---
>>>>    drivers/i2c/i2c-core.c |    2 ++
>>>>    1 files changed, 2 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>>>> index 1cca263..79c6c26 100644
>>>> --- a/drivers/i2c/i2c-core.c
>>>> +++ b/drivers/i2c/i2c-core.c
>>>> @@ -794,6 +794,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>>>>    	mutex_lock(&core_lock);
>>>>    	dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap,
>>>>    				 __process_new_adapter);
>>>> +	if(!dummy)
>>>> +		dummy = 0;
>>>
>>> One word: scripts/checkpatch.pl
>>>
>>> In other news, the above is just plain wrong. First we force people to
>>> read the result of bus_for_each_drv() and then when they do and don't
>>> need the value, gcc complains, so we add one more layer of useless
>>> code, which developers and possibly tools will later wonder and
>>> complain about? I can easily imagine that a static code analyzer would
>>> spot the above code as being a potential bug.
>>>
>>> Let's stop this madness now please.
>>>
>>> Either __must_check goes away from bus_for_each_drv() and from every
>>> other function which raises this problem, or we must disable that new
>>> type of warning gcc 4.6.0 generates. Depends which warnings we value
>>> more, as we can't sanely have both.
>>>
>>
>> That is the crux of the whole thing.  Putting in crap to get rid of the
>> __must_check warning someone obviously wanted to provoke is just plain
>> wrong.
>
> __process_new_adapter() calls i2c_do_add_adapter() which always returns
> 0. Why should I check the return value of bus_for_each_drv() when I
> know it will always be 0 by construction?
>
> Also note that the same function is also called through
> bus_for_each_dev() somewhere else in i2c-core, and there is no warning
> there because bus_for_each_dev() is not marked __must_check. How
> consistent is this? If bus_for_each_dev() is OK without __must_check,
> then I can't see why bus_for_each_drv() wouldn't be.
>

Well, I would advocate removing the __must_check then.


David Daney

Re: [PATCH 6/8]i2c:i2c_core Fix warning: variable 'dummy' set but not used

[Jean Delvare]

On Tue, 15 Jun 2010 09:20:39 -0700, David Daney wrote:
> On 06/15/2010 04:40 AM, Jean Delvare wrote:
> > __process_new_adapter() calls i2c_do_add_adapter() which always returns
> > 0. Why should I check the return value of bus_for_each_drv() when I
> > know it will always be 0 by construction?
> >
> > Also note that the same function is also called through
> > bus_for_each_dev() somewhere else in i2c-core, and there is no warning
> > there because bus_for_each_dev() is not marked __must_check. How
> > consistent is this? If bus_for_each_dev() is OK without __must_check,
> > then I can't see why bus_for_each_drv() wouldn't be.
> 
> Well, I would advocate removing the __must_check then.

I have just sent a patch to LKML doing exactly this.

-- 
Jean Delvare

[PATCH 7/8]ieee1394/sdp2 Fix warning: variable 'unit_characteristics' set but not used

[Justin P. Mattock]

Temporary fix until something is resolved
to fix the below warning:
  CC [M]  drivers/ieee1394/sbp2.o
drivers/ieee1394/sbp2.c: In function 'sbp2_parse_unit_directory':
drivers/ieee1394/sbp2.c:1353:6: warning: variable 'unit_characteristics' set but not used
 Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

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

diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c
index 4565cb5..fcf8bd5 100644
--- a/drivers/ieee1394/sbp2.c
+++ b/drivers/ieee1394/sbp2.c
@@ -1356,6 +1356,8 @@ static void sbp2_parse_unit_directory(struct sbp2_lu *lu,
 
 	management_agent_addr = 0;
 	unit_characteristics = 0;
+	if (!unit_characteristics)
+		unit_characteristics = 0;
 	firmware_revision = SBP2_ROM_VALUE_MISSING;
 	model = ud->flags & UNIT_DIRECTORY_MODEL_ID ?
 				ud->model_id : SBP2_ROM_VALUE_MISSING;
-- 
1.7.1.rc1.21.gf3bd6

[PATCH] ieee1394: sbp2: remove unused code

[Stefan Richter]

which caused gcc 4.6 to warn about
    variable 'unit_characteristics' set but not used.

The underlying problem that was spotted here --- an incomplete
implementation --- is already 50% fixed in drivers/firewire/sbp2.c which
observes mgt_ORB_timeout but not yet ORB_size.

Reported-by: Justin P. Mattock <justinmattock@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/sbp2.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Index: b/drivers/ieee1394/sbp2.c
===================================================================
--- a/drivers/ieee1394/sbp2.c
+++ b/drivers/ieee1394/sbp2.c
@@ -1350,12 +1350,11 @@ static void sbp2_parse_unit_directory(st
 	struct csr1212_keyval *kv;
 	struct csr1212_dentry *dentry;
 	u64 management_agent_addr;
-	u32 unit_characteristics, firmware_revision, model;
+	u32 firmware_revision, model;
 	unsigned workarounds;
 	int i;
 
 	management_agent_addr = 0;
-	unit_characteristics = 0;
 	firmware_revision = SBP2_ROM_VALUE_MISSING;
 	model = ud->flags & UNIT_DIRECTORY_MODEL_ID ?
 				ud->model_id : SBP2_ROM_VALUE_MISSING;
@@ -1372,17 +1371,15 @@ static void sbp2_parse_unit_directory(st
 				lu->lun = ORB_SET_LUN(kv->value.immediate);
 			break;
 
-		case SBP2_UNIT_CHARACTERISTICS_KEY:
-			/* FIXME: This is ignored so far.
-			 * See SBP-2 clause 7.4.8. */
-			unit_characteristics = kv->value.immediate;
-			break;
 
 		case SBP2_FIRMWARE_REVISION_KEY:
 			firmware_revision = kv->value.immediate;
 			break;
 
 		default:
+			/* FIXME: Check for SBP2_UNIT_CHARACTERISTICS_KEY
+			 * mgt_ORB_timeout and ORB_size, SBP-2 clause 7.4.8. */
+
 			/* FIXME: Check for SBP2_DEVICE_TYPE_AND_LUN_KEY.
 			 * Its "ordered" bit has consequences for command ORB
 			 * list handling. See SBP-2 clauses 4.6, 7.4.11, 10.2 */

-- 
Stefan Richter
-=====-==-=- -==- -===-
http://arcgraph.de/sr/

Re: [PATCH] ieee1394: sbp2: remove unused code

[Justin P. Mattock]

On 06/14/2010 02:44 PM, Stefan Richter wrote:
> which caused gcc 4.6 to warn about
>      variable 'unit_characteristics' set but not used.
>
> The underlying problem that was spotted here --- an incomplete
> implementation --- is already 50% fixed in drivers/firewire/sbp2.c which
> observes mgt_ORB_timeout but not yet ORB_size.
>
> Reported-by: Justin P. Mattock<justinmattock@gmail.com>
> Signed-off-by: Stefan Richter<stefanr@s5r6.in-berlin.de>
> ---
>   drivers/ieee1394/sbp2.c |   11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
>
> Index: b/drivers/ieee1394/sbp2.c
> ===================================================================
> --- a/drivers/ieee1394/sbp2.c
> +++ b/drivers/ieee1394/sbp2.c
> @@ -1350,12 +1350,11 @@ static void sbp2_parse_unit_directory(st
>   	struct csr1212_keyval *kv;
>   	struct csr1212_dentry *dentry;
>   	u64 management_agent_addr;
> -	u32 unit_characteristics, firmware_revision, model;
> +	u32 firmware_revision, model;
>   	unsigned workarounds;
>   	int i;
>
>   	management_agent_addr = 0;
> -	unit_characteristics = 0;
>   	firmware_revision = SBP2_ROM_VALUE_MISSING;
>   	model = ud->flags&  UNIT_DIRECTORY_MODEL_ID ?
>   				ud->model_id : SBP2_ROM_VALUE_MISSING;
> @@ -1372,17 +1371,15 @@ static void sbp2_parse_unit_directory(st
>   				lu->lun = ORB_SET_LUN(kv->value.immediate);
>   			break;
>
> -		case SBP2_UNIT_CHARACTERISTICS_KEY:
> -			/* FIXME: This is ignored so far.
> -			 * See SBP-2 clause 7.4.8. */
> -			unit_characteristics = kv->value.immediate;
> -			break;
>
>   		case SBP2_FIRMWARE_REVISION_KEY:
>   			firmware_revision = kv->value.immediate;
>   			break;
>
>   		default:
> +			/* FIXME: Check for SBP2_UNIT_CHARACTERISTICS_KEY
> +			 * mgt_ORB_timeout and ORB_size, SBP-2 clause 7.4.8. */
> +
>   			/* FIXME: Check for SBP2_DEVICE_TYPE_AND_LUN_KEY.
>   			 * Its "ordered" bit has consequences for command ORB
>   			 * list handling. See SBP-2 clauses 4.6, 7.4.11, 10.2 */
>


perfect!! compiled without any warning
with that one..
thanks for the reply and patch..

FWIW if you have time there's these guys as well
that I never looked at:

   CC [M]  drivers/firewire/core-transaction.o
drivers/firewire/core-transaction.c: In function 'fw_core_handle_response':
drivers/firewire/core-transaction.c:835:21: warning: variable 
'destination' set but not used
   CC [M]  drivers/firewire/ohci.o

   CC [M]  drivers/ieee1394/raw1394.o
drivers/ieee1394/raw1394.c: In function 'arm_write':
drivers/ieee1394/raw1394.c:1018:39: warning: variable 'length_conflict' 
set but not used
drivers/ieee1394/raw1394.c: In function 'arm_lock64':
drivers/ieee1394/raw1394.c:1373:11: warning: 'old' may be used 
uninitialized in this function
drivers/ieee1394/raw1394.c: In function 'arm_lock':
drivers/ieee1394/raw1394.c:1155:12: warning: 'old' may be used 
uninitialized in this function


  CC [M]  drivers/ieee1394/dv1394.o
drivers/ieee1394/dv1394.c: In function 'frame_prepare':
drivers/ieee1394/dv1394.c:613:15: warning: variable 'ts_off' set but not 
used
drivers/ieee1394/dv1394.c: In function 'ir_tasklet_func':
drivers/ieee1394/dv1394.c:2007:22: warning: variable 'packet_time' set 
but not used
drivers/ieee1394/dv1394.c: In function 'dv1394_host_reset':
drivers/ieee1394/dv1394.c:2323:18: warning: variable 'ohci' set but not used
   CC [M]  drivers/ieee1394/eth1394.o
drivers/ieee1394/eth1394.c: In function 'ether1394_iso':
drivers/ieee1394/eth1394.c:1261:23: warning: variable 'priv' set but not 
used
   LD      drivers/ieee802154/built-in.o


I can test and see!!

Justin P. Mattock

(no subject)

[Stefan Richter]

which caused gcc 4.6 to warn about
    variable 'destination' set but not used.

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

Since the hardware ensures that we receive only response packets with
proper destination node ID (in a given bus generation), we have no use
for destination here in the core as well as in upper layers.

(This is different with request packets.  There we pass destination node
ID to upper layers because they may for example need to check whether
this was an unicast or broadcast request.)

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/core-transaction.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Index: b/drivers/firewire/core-transaction.c
===================================================================
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -881,13 +881,12 @@ void fw_core_handle_response(struct fw_c
 	unsigned long flags;
 	u32 *data;
 	size_t data_length;
-	int tcode, tlabel, destination, source, rcode;
+	int tcode, tlabel, source, rcode;
 
-	tcode       = HEADER_GET_TCODE(p->header[0]);
-	tlabel      = HEADER_GET_TLABEL(p->header[0]);
-	destination = HEADER_GET_DESTINATION(p->header[0]);
-	source      = HEADER_GET_SOURCE(p->header[1]);
-	rcode       = HEADER_GET_RCODE(p->header[1]);
+	tcode	= HEADER_GET_TCODE(p->header[0]);
+	tlabel	= HEADER_GET_TLABEL(p->header[0]);
+	source	= HEADER_GET_SOURCE(p->header[1]);
+	rcode	= HEADER_GET_RCODE(p->header[1]);
 
 	spin_lock_irqsave(&card->lock, flags);
 	list_for_each_entry(t, &card->transaction_list, link) {

-- 
Stefan Richter
-=====-==-=- -==- -====
http://arcgraph.de/sr/

Re:

[Justin P. Mattock]

On 06/14/2010 04:22 PM, Stefan Richter wrote:
> which caused gcc 4.6 to warn about
>      variable 'destination' set but not used.
>
> Reported-by: Justin P. Mattock<justinmattock@gmail.com>
>
> Since the hardware ensures that we receive only response packets with
> proper destination node ID (in a given bus generation), we have no use
> for destination here in the core as well as in upper layers.
>
> (This is different with request packets.  There we pass destination node
> ID to upper layers because they may for example need to check whether
> this was an unicast or broadcast request.)
>
> Signed-off-by: Stefan Richter<stefanr@s5r6.in-berlin.de>
> ---
>   drivers/firewire/core-transaction.c |   11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
>
> Index: b/drivers/firewire/core-transaction.c
> ===================================================================
> --- a/drivers/firewire/core-transaction.c
> +++ b/drivers/firewire/core-transaction.c
> @@ -881,13 +881,12 @@ void fw_core_handle_response(struct fw_c
>   	unsigned long flags;
>   	u32 *data;
>   	size_t data_length;
> -	int tcode, tlabel, destination, source, rcode;
> +	int tcode, tlabel, source, rcode;
>
> -	tcode       = HEADER_GET_TCODE(p->header[0]);
> -	tlabel      = HEADER_GET_TLABEL(p->header[0]);
> -	destination = HEADER_GET_DESTINATION(p->header[0]);
> -	source      = HEADER_GET_SOURCE(p->header[1]);
> -	rcode       = HEADER_GET_RCODE(p->header[1]);
> +	tcode	= HEADER_GET_TCODE(p->header[0]);
> +	tlabel	= HEADER_GET_TLABEL(p->header[0]);
> +	source	= HEADER_GET_SOURCE(p->header[1]);
> +	rcode	= HEADER_GET_RCODE(p->header[1]);
>
>   	spin_lock_irqsave(&card->lock, flags);
>   	list_for_each_entry(t,&card->transaction_list, link) {
>


built good.. here's what I see now:

   LD      kernel/built-in.o
   CC [M]  fs/reiserfs/stree.o
   LD [M]  fs/reiserfs/reiserfs.o
   CC [M]  drivers/firewire/core-transaction.o
   LD [M]  drivers/firewire/firewire-core.o
   LD [M]  drivers/firewire/firewire-ohci.o
   LD [M]  drivers/firewire/firewire-sbp2.o
   CC [M]  drivers/ieee1394/sbp2.o
   CC [M]  drivers/net/wireless/hostap/hostap_80211_rx.o
   CC [M]  drivers/net/wireless/hostap/hostap_80211_tx.o
   CC [M]  drivers/net/wireless/hostap/hostap_ap.o

nice and clean!!

Reported-and-Tested-By: Justin P. Mattock <justinmattock@gmail.com>

Justin P. Mattock

Re: (no subject)

[Stefan Richter]

On 15 Jun, Stefan Richter wrote:
> which caused gcc 4.6 to warn about
>     variable 'destination' set but not used.
...
>  drivers/firewire/core-transaction.c |   11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
...

Subject was meant to be [PATCH] firewire: core: remove unused variable.
-- 
Stefan Richter
-=====-==-=- -==- -====
http://arcgraph.de/sr/

[PATCH] ieee1394: remove unused variables

[Stefan Richter]

which caused gcc 4.6 to warn about
    variable 'XYZ' set but not used.

sbp2.c, unit_characteristics:

The underlying problem which was spotted here --- an incomplete
implementation --- is already 50% fixed in drivers/firewire/sbp2.c which
observes mgt_ORB_timeout but not yet ORB_size.

raw1394.c, length_conflict; dv1394.c, ts_off:

Impossible to tell why these variables are there.  We can safely remove
them though because we don't need a compiler warning to realize that we
are dealing with (at least stylistically) flawed code here.

dv1394.c, packet_time:

This was used in debug macro that is only compiled in with
DV1394_DEBUG_LEVEL >= 2 defined at compile-time.  Just drop it since
nobody debugs dv1394 anymore.  Avoids noise in regular kernel builds.

dv1394.c, ohci; eth1394.c, priv:

These variables clearly can go away.  Somebody wanted to use them but
then didn't (or not anymore).

Note, all of this code is considered to be at its end of life and is
thus not really meant to receive janitorial updates anymore.  But if we
can easily remove noisy warnings from kernel builds, we should.

Reported-by: Justin P. Mattock <justinmattock@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---

Incorporates patch "ieee1394: sbp2: remove unused code".

 drivers/ieee1394/dv1394.c  |   14 ++------------
 drivers/ieee1394/eth1394.c |    3 ---
 drivers/ieee1394/raw1394.c |    3 +--
 drivers/ieee1394/sbp2.c    |   11 ++++-------
 4 files changed, 7 insertions(+), 24 deletions(-)

Index: b/drivers/ieee1394/sbp2.c
===================================================================
--- a/drivers/ieee1394/sbp2.c
+++ b/drivers/ieee1394/sbp2.c
@@ -1350,12 +1350,11 @@ static void sbp2_parse_unit_directory(st
 	struct csr1212_keyval *kv;
 	struct csr1212_dentry *dentry;
 	u64 management_agent_addr;
-	u32 unit_characteristics, firmware_revision, model;
+	u32 firmware_revision, model;
 	unsigned workarounds;
 	int i;
 
 	management_agent_addr = 0;
-	unit_characteristics = 0;
 	firmware_revision = SBP2_ROM_VALUE_MISSING;
 	model = ud->flags & UNIT_DIRECTORY_MODEL_ID ?
 				ud->model_id : SBP2_ROM_VALUE_MISSING;
@@ -1372,17 +1371,15 @@ static void sbp2_parse_unit_directory(st
 				lu->lun = ORB_SET_LUN(kv->value.immediate);
 			break;
 
-		case SBP2_UNIT_CHARACTERISTICS_KEY:
-			/* FIXME: This is ignored so far.
-			 * See SBP-2 clause 7.4.8. */
-			unit_characteristics = kv->value.immediate;
-			break;
 
 		case SBP2_FIRMWARE_REVISION_KEY:
 			firmware_revision = kv->value.immediate;
 			break;
 
 		default:
+			/* FIXME: Check for SBP2_UNIT_CHARACTERISTICS_KEY
+			 * mgt_ORB_timeout and ORB_size, SBP-2 clause 7.4.8. */
+
 			/* FIXME: Check for SBP2_DEVICE_TYPE_AND_LUN_KEY.
 			 * Its "ordered" bit has consequences for command ORB
 			 * list handling. See SBP-2 clauses 4.6, 7.4.11, 10.2 */
Index: b/drivers/ieee1394/raw1394.c
===================================================================
--- a/drivers/ieee1394/raw1394.c
+++ b/drivers/ieee1394/raw1394.c
@@ -1015,7 +1015,7 @@ static int arm_write(struct hpsb_host *h
 	struct arm_addr *arm_addr = NULL;
 	struct arm_request *arm_req = NULL;
 	struct arm_response *arm_resp = NULL;
-	int found = 0, size = 0, rcode = -1, length_conflict = 0;
+	int found = 0, size = 0, rcode = -1;
 	struct arm_request_response *arm_req_resp = NULL;
 
 	DBGMSG("arm_write called by node: %X "
@@ -1054,7 +1054,6 @@ static int arm_write(struct hpsb_host *h
 	}
 	if (arm_addr->rec_length < length) {
 		DBGMSG("arm_write blocklength too big -> rcode_data_error");
-		length_conflict = 1;
 		rcode = RCODE_DATA_ERROR;	/* hardware error, data is unavailable */
 	}
 	if (rcode == -1) {
Index: b/drivers/ieee1394/dv1394.c
===================================================================
--- a/drivers/ieee1394/dv1394.c
+++ b/drivers/ieee1394/dv1394.c
@@ -610,7 +610,7 @@ static void frame_prepare(struct video_c
 	} else {
 
 		u32 transmit_sec, transmit_cyc;
-		u32 ts_cyc, ts_off;
+		u32 ts_cyc;
 
 		/* DMA is stopped, so this is the very first frame */
 		video->active_frame = this_frame;
@@ -636,7 +636,6 @@ static void frame_prepare(struct video_c
 		transmit_sec += transmit_cyc/8000;
 		transmit_cyc %= 8000;
 
-		ts_off = ct_off;
 		ts_cyc = transmit_cyc + 3;
 		ts_cyc %= 8000;
 
@@ -2004,7 +2003,7 @@ static void ir_tasklet_func(unsigned lon
 
 		int sof=0; /* start-of-frame flag */
 		struct frame *f;
-		u16 packet_length, packet_time;
+		u16 packet_length;
 		int i, dbc=0;
 		struct DMA_descriptor_block *block = NULL;
 		u16 xferstatus;
@@ -2024,11 +2023,6 @@ static void ir_tasklet_func(unsigned lon
 						sizeof(struct packet));
 
 			packet_length = le16_to_cpu(p->data_length);
-			packet_time   = le16_to_cpu(p->timestamp);
-
-			irq_printk("received packet %02d, timestamp=%04x, length=%04x, sof=%02x%02x\n", video->current_packet,
-				   packet_time, packet_length,
-				   p->data[0], p->data[1]);
 
 			/* get the descriptor based on packet_buffer cursor */
 			f = video->frames[video->current_packet / MAX_PACKETS];
@@ -2320,7 +2314,6 @@ static void dv1394_add_host(struct hpsb_
 
 static void dv1394_host_reset(struct hpsb_host *host)
 {
-	struct ti_ohci *ohci;
 	struct video_card *video = NULL, *tmp_vid;
 	unsigned long flags;
 
@@ -2328,9 +2321,6 @@ static void dv1394_host_reset(struct hps
 	if (strcmp(host->driver->name, OHCI1394_DRIVER_NAME))
 		return;
 
-	ohci = (struct ti_ohci *)host->hostdata;
-
-
 	/* find the corresponding video_cards */
 	spin_lock_irqsave(&dv1394_cards_lock, flags);
 	list_for_each_entry(tmp_vid, &dv1394_cards, list) {
Index: b/drivers/ieee1394/eth1394.c
===================================================================
--- a/drivers/ieee1394/eth1394.c
+++ b/drivers/ieee1394/eth1394.c
@@ -1258,7 +1258,6 @@ static void ether1394_iso(struct hpsb_is
 	char *buf;
 	struct eth1394_host_info *hi;
 	struct net_device *dev;
-	struct eth1394_priv *priv;
 	unsigned int len;
 	u32 specifier_id;
 	u16 source_id;
@@ -1288,8 +1287,6 @@ static void ether1394_iso(struct hpsb_is
 			       (be32_to_cpu(data[1]) & 0xff000000) >> 24;
 		source_id = be32_to_cpu(data[0]) >> 16;
 
-		priv = netdev_priv(dev);
-
 		if (info->channel != (iso->host->csr.broadcast_channel & 0x3f)
 		    || specifier_id != ETHER1394_GASP_SPECIFIER_ID) {
 			/* This packet is not for us */


-- 
Stefan Richter
-=====-==-=- -==- -====
http://arcgraph.de/sr/

Re: [PATCH] ieee1394: sbp2: remove unused code

[Stefan Richter]

On 14 Jun, Justin P. Mattock wrote:
> drivers/ieee1394/raw1394.c: In function 'arm_lock64':
> drivers/ieee1394/raw1394.c:1373:11: warning: 'old' may be used 
> uninitialized in this function
> drivers/ieee1394/raw1394.c: In function 'arm_lock':
> drivers/ieee1394/raw1394.c:1155:12: warning: 'old' may be used 
> uninitialized in this function

Thanks.  I briefly looked at this and am not sure if this is real or a
false positive.  This code is old crud that is really hard to read.

I will look at it again at a more convenient time of day.
-- 
Stefan Richter
-=====-==-=- -==- -====
http://arcgraph.de/sr/

Re: [PATCH] ieee1394: sbp2: remove unused code

[Justin P. Mattock]

On 06/14/2010 05:05 PM, Stefan Richter wrote:
> On 14 Jun, Justin P. Mattock wrote:
>> drivers/ieee1394/raw1394.c: In function 'arm_lock64':
>> drivers/ieee1394/raw1394.c:1373:11: warning: 'old' may be used
>> uninitialized in this function
>> drivers/ieee1394/raw1394.c: In function 'arm_lock':
>> drivers/ieee1394/raw1394.c:1155:12: warning: 'old' may be used
>> uninitialized in this function
>
> Thanks.  I briefly looked at this and am not sure if this is real or a
> false positive.  This code is old crud that is really hard to read.
>
> I will look at it again at a more convenient time of day.


I dont think you need to buddy!! just applied all
patches you sent..
she looks like this now:

   CC [M]  drivers/firewire/core-card.o
   CC [M]  drivers/firewire/core-cdev.o
   CC [M]  drivers/firewire/core-device.o
   CC [M]  drivers/firewire/core-iso.o
   CC [M]  drivers/firewire/core-topology.o
   CC [M]  drivers/firewire/core-transaction.o
   CC [M]  drivers/firewire/ohci.o
   CC [M]  drivers/firewire/sbp2.o
   LD [M]  drivers/firewire/firewire-core.o
   LD [M]  drivers/firewire/firewire-ohci.o
   LD [M]  drivers/firewire/firewire-sbp2.o
   CC      drivers/firmware/dmi_scan.o
   CC      drivers/firmware/efivars.o
   CC      drivers/firmware/dmi-id.o
   CC      drivers/firmware/memmap.o
   LD      drivers/firmware/built-in.o

she's clean as a whistle!!

only concern now is the
hardware and driver functioning properly
i.g. I haven't installed the kernel
(if it even makes a difference)
just compiling and looking at warning messages.

cheers,

Justin P. Mattock

Re: [PATCH 7/8]ieee1394/sdp2 Fix warning: variable 'unit_characteristics' set but not used

[Jean Delvare]

On Mon, 14 Jun 2010 13:26:47 -0700, Justin P. Mattock wrote:
> Temporary fix until something is resolved

This is wrong by design, sorry. Warnings aren't blocking, and thus need
no "temporary fix". Such temporary fixes would be only hiding the
warning, cancelling the good work of gcc developers. Nack nack nack.

> to fix the below warning:
>   CC [M]  drivers/ieee1394/sbp2.o
> drivers/ieee1394/sbp2.c: In function 'sbp2_parse_unit_directory':
> drivers/ieee1394/sbp2.c:1353:6: warning: variable 'unit_characteristics' set but not used
>  Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
> 
> ---
>  drivers/ieee1394/sbp2.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c
> index 4565cb5..fcf8bd5 100644
> --- a/drivers/ieee1394/sbp2.c
> +++ b/drivers/ieee1394/sbp2.c
> @@ -1356,6 +1356,8 @@ static void sbp2_parse_unit_directory(struct sbp2_lu *lu,
>  
>  	management_agent_addr = 0;
>  	unit_characteristics = 0;
> +	if (!unit_characteristics)
> +		unit_characteristics = 0;
>  	firmware_revision = SBP2_ROM_VALUE_MISSING;
>  	model = ud->flags & UNIT_DIRECTORY_MODEL_ID ?
>  				ud->model_id : SBP2_ROM_VALUE_MISSING;


-- 
Jean Delvare

Re: [PATCH 7/8]ieee1394/sdp2 Fix warning: variable 'unit_characteristics' set but not used

[Justin P. Mattock]

On 06/15/2010 04:38 AM, Jean Delvare wrote:
> On Mon, 14 Jun 2010 13:26:47 -0700, Justin P. Mattock wrote:
>> Temporary fix until something is resolved
>
> This is wrong by design, sorry. Warnings aren't blocking, and thus need
> no "temporary fix". Such temporary fixes would be only hiding the
> warning, cancelling the good work of gcc developers. Nack nack nack.
>

o.k.

>> to fix the below warning:
>>    CC [M]  drivers/ieee1394/sbp2.o
>> drivers/ieee1394/sbp2.c: In function 'sbp2_parse_unit_directory':
>> drivers/ieee1394/sbp2.c:1353:6: warning: variable 'unit_characteristics' set but not used
>>   Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
>>
>> ---
>>   drivers/ieee1394/sbp2.c |    2 ++
>>   1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c
>> index 4565cb5..fcf8bd5 100644
>> --- a/drivers/ieee1394/sbp2.c
>> +++ b/drivers/ieee1394/sbp2.c
>> @@ -1356,6 +1356,8 @@ static void sbp2_parse_unit_directory(struct sbp2_lu *lu,
>>
>>   	management_agent_addr = 0;
>>   	unit_characteristics = 0;
>> +	if (!unit_characteristics)
>> +		unit_characteristics = 0;
>>   	firmware_revision = SBP2_ROM_VALUE_MISSING;
>>   	model = ud->flags&  UNIT_DIRECTORY_MODEL_ID ?
>>   				ud->model_id : SBP2_ROM_VALUE_MISSING;
>
>

Thanks for the response and info on this.

Justin P. Mattock

[PATCH 8/8]tuners:tuner-simple Fix warning: variable 'tun' set but not used

[Justin P. Mattock]

not sure if this is correct or not for 
fixing this warning:
  CC [M]  drivers/media/common/tuners/tuner-simple.o
drivers/media/common/tuners/tuner-simple.c: In function 'simple_set_tv_freq':
drivers/media/common/tuners/tuner-simple.c:548:20: warning: variable 'tun' set but not used

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

---
 drivers/media/common/tuners/tuner-simple.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/media/common/tuners/tuner-simple.c b/drivers/media/common/tuners/tuner-simple.c
index 8abbcc5..4465b99 100644
--- a/drivers/media/common/tuners/tuner-simple.c
+++ b/drivers/media/common/tuners/tuner-simple.c
@@ -545,14 +545,12 @@ static int simple_set_tv_freq(struct dvb_frontend *fe,
 	struct tuner_simple_priv *priv = fe->tuner_priv;
 	u8 config, cb;
 	u16 div;
-	struct tunertype *tun;
 	u8 buffer[4];
 	int rc, IFPCoff, i;
 	enum param_type desired_type;
 	struct tuner_params *t_params;
 
-	tun = priv->tun;
-
+	
 	/* IFPCoff = Video Intermediate Frequency - Vif:
 		940  =16*58.75  NTSC/J (Japan)
 		732  =16*45.75  M/N STD
-- 
1.7.1.rc1.21.gf3bd6

Re: [PATCH 8/8]tuners:tuner-simple Fix warning: variable 'tun' set but not used

[Mauro Carvalho Chehab]

Em 14-06-2010 23:26, Justin P. Mattock escreveu:
> not sure if this is correct or not for 
> fixing this warning:
>   CC [M]  drivers/media/common/tuners/tuner-simple.o
> drivers/media/common/tuners/tuner-simple.c: In function 'simple_set_tv_freq':
> drivers/media/common/tuners/tuner-simple.c:548:20: warning: variable 'tun' set but not used
> 
>  Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
> 
> ---
>  drivers/media/common/tuners/tuner-simple.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/common/tuners/tuner-simple.c b/drivers/media/common/tuners/tuner-simple.c
> index 8abbcc5..4465b99 100644
> --- a/drivers/media/common/tuners/tuner-simple.c
> +++ b/drivers/media/common/tuners/tuner-simple.c
> @@ -545,14 +545,12 @@ static int simple_set_tv_freq(struct dvb_frontend *fe,
>  	struct tuner_simple_priv *priv = fe->tuner_priv;
>  	u8 config, cb;
>  	u16 div;
> -	struct tunertype *tun;
>  	u8 buffer[4];
>  	int rc, IFPCoff, i;
>  	enum param_type desired_type;
>  	struct tuner_params *t_params;
>  
> -	tun = priv->tun;
> -
> +	
Why are you adding an extra blank line here? Except for that, the patch
looks sane.

>  	/* IFPCoff = Video Intermediate Frequency - Vif:
>  		940  =16*58.75  NTSC/J (Japan)
>  		732  =16*45.75  M/N STD

Re: [PATCH 8/8]tuners:tuner-simple Fix warning: variable 'tun' set but not used

[Justin P. Mattock]

On 06/14/2010 10:16 PM, Mauro Carvalho Chehab wrote:
>
>
> Em 14-06-2010 23:26, Justin P. Mattock escreveu:
>> not sure if this is correct or not for
>> fixing this warning:
>>    CC [M]  drivers/media/common/tuners/tuner-simple.o
>> drivers/media/common/tuners/tuner-simple.c: In function 'simple_set_tv_freq':
>> drivers/media/common/tuners/tuner-simple.c:548:20: warning: variable 'tun' set but not used
>>
>>   Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
>>
>> ---
>>   drivers/media/common/tuners/tuner-simple.c |    4 +---
>>   1 files changed, 1 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/common/tuners/tuner-simple.c b/drivers/media/common/tuners/tuner-simple.c
>> index 8abbcc5..4465b99 100644
>> --- a/drivers/media/common/tuners/tuner-simple.c
>> +++ b/drivers/media/common/tuners/tuner-simple.c
>> @@ -545,14 +545,12 @@ static int simple_set_tv_freq(struct dvb_frontend *fe,
>>   	struct tuner_simple_priv *priv = fe->tuner_priv;
>>   	u8 config, cb;
>>   	u16 div;
>> -	struct tunertype *tun;
>>   	u8 buffer[4];
>>   	int rc, IFPCoff, i;
>>   	enum param_type desired_type;
>>   	struct tuner_params *t_params;
>>
>> -	tun = priv->tun;
>> -
>> +	
> Why are you adding an extra blank line here? Except for that, the patch
> looks sane.
>

I think I was doing something wrong when creating these patches. i.g.
I just hightlight the code then move the cursor highlight all the way to 
the right before pressing "x". normally would be o.k. but for some 
reason seems to be doing this. found if I highlight left to ; (or the 
end of the code I want to delete) then git commit creates the patch 
properly.

>>   	/* IFPCoff = Video Intermediate Frequency - Vif:
>>   		940  =16*58.75  NTSC/J (Japan)
>>   		732  =16*45.75  M/N STD
>
>

I'll resend this.

Thanks for having a look.

Justin P. Mattock

Re: [PATCH 8/8]tuners:tuner-simple Fix warning: variable 'tun' set but not used

[Justin P. Mattock]

On 06/14/2010 10:16 PM, Mauro Carvalho Chehab wrote:
>
>
> Em 14-06-2010 23:26, Justin P. Mattock escreveu:
>> not sure if this is correct or not for
>> fixing this warning:
>>    CC [M]  drivers/media/common/tuners/tuner-simple.o
>> drivers/media/common/tuners/tuner-simple.c: In function 'simple_set_tv_freq':
>> drivers/media/common/tuners/tuner-simple.c:548:20: warning: variable 'tun' set but not used
>>
>>   Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
>>
>> ---
>>   drivers/media/common/tuners/tuner-simple.c |    4 +---
>>   1 files changed, 1 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/common/tuners/tuner-simple.c b/drivers/media/common/tuners/tuner-simple.c
>> index 8abbcc5..4465b99 100644
>> --- a/drivers/media/common/tuners/tuner-simple.c
>> +++ b/drivers/media/common/tuners/tuner-simple.c
>> @@ -545,14 +545,12 @@ static int simple_set_tv_freq(struct dvb_frontend *fe,
>>   	struct tuner_simple_priv *priv = fe->tuner_priv;
>>   	u8 config, cb;
>>   	u16 div;
>> -	struct tunertype *tun;
>>   	u8 buffer[4];
>>   	int rc, IFPCoff, i;
>>   	enum param_type desired_type;
>>   	struct tuner_params *t_params;
>>
>> -	tun = priv->tun;
>> -
>> +	
> Why are you adding an extra blank line here? Except for that, the patch
> looks sane.
>
>>   	/* IFPCoff = Video Intermediate Frequency - Vif:
>>   		940  =16*58.75  NTSC/J (Japan)
>>   		732  =16*45.75  M/N STD
>
>

o.k. resent this.. I ended up doing
a git reset do make sure things dont get
funky etc..

Justin P. Mattock