Skip to contentSkip to conversation

check u32 < 0 in cpufreq-nforce2

PATCH In review Eric Sesterhenn / snakebyte 3 participants 4 messages Dec 14, 2005 Kernel janitors

lore
Share & tools
lore

Eric Sesterhenn / snakebyte

author
#0 lore raw

PATCH 1/1 check u32 < 0 in cpufreq-nforce2

Hi,

arch/i386/kernel/cpu/cpufreq/cpufreq-nforce2.c declares pll as u32 which is unsigned, and checks it agains < 0. This patch changes it from u32 to s32 so the check makes sense, nforce2_write_pll() and nforce2_calc_pll() also expect/return a signed int.

Signed-off-by Eric Sesterhenn <[email protected]>
i386/kernel/cpu/cpufreq/cpufreq-nforce2.c renamed from i386/kernel/cpu/cpufreq/cpufreq-nforce2.c.orig +2 -1
@@ -177,7 +177,8 @@ static unsigned int nforce2_fsb_read(int
  */
 static int nforce2_set_fsb(unsigned int fsb)
 {
-	u32 pll, temp = 0;
+	u32 temp = 0;
+	s32 pll;
 	unsigned int tfsb;
 	int diff;

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#1 lore raw

i386/kernel/cpu/cpufreq/cpufreq-nforce2.c 2005-12-14 13:23:40.000000000 +0100

Quoted 12 lines · Eric Sesterhenn / snakebyte

Hi,

arch/i386/kernel/cpu/cpufreq/cpufreq-nforce2.c declares pll as u32 which is unsigned, and checks it agains < 0. This patch changes it from u32 to s32 so the check makes sense, nforce2_write_pll() and nforce2_calc_pll() also expect/return a signed int.

Signed-off-by Eric Sesterhenn <[email protected]>
i386/kernel/cpu/cpufreq/cpufreq-nforce2.c 2005-12-14 13:23:40.000000000 +0100 renamed from i386/kernel/cpu/cpufreq/cpufreq-nforce2.c.orig 2005-12-14 13:19:43.000000000 +0100 +0 -0

Please make patches that can be applied with patch -p1 that is, run diff from the directory holding the kernel source dir, not from within the kernel source dir itself. Also, unless creating your diff that way makes it obvious what version of the kernel you are patching, please state the version in your email.

i386/kernel/cpu/cpufreq/cpufreq-nforce2.c 2005-12-14 13:23:40.000000000 +0100 static unsigned int nforce2_fsb_read(int · line 184

Quoted 10 lines
i386/kernel/cpu/cpufreq/cpufreq-nforce2.c 2005-12-14 13:23:40.000000000 +0100 renamed from i386/kernel/cpu/cpufreq/cpufreq-nforce2.c.orig 2005-12-14 13:19:43.000000000 +0100 +2 -1
@@ -177,7 +177,8 @@ static unsigned int nforce2_fsb_read(int
  */
 static int nforce2_set_fsb(unsigned int fsb)
 {
-       u32 pll, temp = 0;
+       u32 temp = 0;
+       s32 pll;
        unsigned int tfsb;
        int diff;

nforce2_calc_pll() which is used to obtain the value stored in the pll variable returns a plain 'int', and nforce2_write_pll() which the value is later passed to also expects a plain 'int', so why not make the type of pll a plain int instead of s32 ??

Other than that, I agree with you that the variable needs to be signed since nforce2_calc_pll() returns -1 on failure.

--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

author
#2 lore raw
In reply to Eric Sesterhenn / snakebyte

hi,

thanks for the reply

Quoted 5 lines

Please make patches that can be applied with patch -p1 that is, run diff from the directory holding the kernel source dir, not from within the kernel source dir itself. Also, unless creating your diff that way makes it obvious what version of the kernel you are patching, please state the version in your email.

ok, rediffed, kernel ist 2.6.16-rc5-git2

Quoted 4 lines

nforce2_calc_pll() which is used to obtain the value stored in the pll variable returns a plain 'int', and nforce2_write_pll() which the value is later passed to also expects a plain 'int', so why not make the type of pll a plain int instead of s32 ??

i wasnt brave enough, since i am not fully aware of the differences between s32 and int. changed it to int.

Quoted 2 lines

Other than that, I agree with you that the variable needs to be signed since nforce2_calc_pll() returns -1 on failure.

so such cleanups are ok? the icc compiler spits out some more warnings regarding this ( not all seem to be a problem ).

Signed-off-by Eric Sesterhenn <[email protected]>
arch/i386/kernel/cpu/cpufreq/cpufreq-nforce2.c renamed from arch/i386/kernel/cpu/cpufreq/cpufreq-nforce2.c.orig +2 -2
@@ -177,9 +177,9 @@ static unsigned int nforce2_fsb_read(int
  */
 static int nforce2_set_fsb(unsigned int fsb)
 {
-	u32 pll, temp = 0;
+	u32 temp = 0;
 	unsigned int tfsb;
-	int diff;
+	int diff, pll;
 
 	if ((fsb > max_fsb) || (fsb < NFORCE2_MIN_FSB)) {
 		printk(KERN_ERR "cpufreq: FSB %d is out of range!\n", fsb);

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#3 lore raw
In reply to Eric Sesterhenn / snakebyte
Quoted 9 lines · Eric Sesterhenn / snakebyte

Hi,

arch/i386/kernel/cpu/cpufreq/cpufreq-nforce2.c declares pll as u32 which is unsigned, and checks it agains < 0. This patch changes it from u32 to s32 so the check makes sense, nforce2_write_pll() and nforce2_calc_pll() also expect/return a signed int.

Signed-off-by Eric Sesterhenn <[email protected]>

Already fixed in cpufreq.git / -mm
Will go to Linus post 2.6.15

Dave

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors