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
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; }
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.
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
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;
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; }
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; }
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
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.
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
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);
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.
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:
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);
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?
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
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.
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
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
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
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);
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. Mattockjustinmattock@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;-- 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");
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
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);
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;
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;
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. Mattockjustinmattock@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
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. Mattockjustinmattock@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
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;
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
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
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. Mattockjustinmattock@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
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. Mattockjustinmattock@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
dri-devel@lists.freedesktop.org
-
David Daney -
Edward Shishkin -
Gustavo F. Padovan -
Jean Delvare -
Justin P. Mattock -
Mauro Carvalho Chehab -
Nick Bowler -
Peter Stuge -
Sergey V. -
Stefan Richter -
Valdis.Kletnieks＠vt.edu