[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20071230205958.GB7883@cvg.cvg>
Date: Sun, 30 Dec 2007 23:59:58 +0300
From: Cyrill Gorcunov <gorcunov@...il.com>
To: Ingo Molnar <mingo@...e.hu>
Cc: "H. Peter Anvin" <hpa@...or.com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [x86] is checkpatch.pl broken
[Ingo Molnar - Sun, Dec 30, 2007 at 06:22:50PM +0100]
|
| * Cyrill Gorcunov <gorcunov@...il.com> wrote:
|
| > orig:
| > mbr_base = (buf_base+sector_size-1) & ~(sector_size-1);
| > new (could be):
| > mbr_base = (buf_base + sector_size - 1) & ~(sector_size - 1);
| >
| > Is a new version that bad?
|
| it's certainly acceptable as newly introduced code but only borderline
| better than the original code. I'd suggest to stick to the problem areas
| that checkpatch.pl complains about at the moment - we have really
| obvious bad looking pieces of code that checkpatch.pl reports, and going
| after the borderline cases will only result in coding-style lawyering
| and flamewars, not any genuine increase in code quality ;-)
|
| for example:
|
| arch/x86/kernel/bootflag.c:
|
| total: 19 errors, 2 warnings, 98 lines checked
|
| or:
|
| arch/x86/kernel/apm_32.c:
|
| total: 56 errors, 31 warnings, 2402 lines checked
|
| and once we have nothing but the borderline cases and if we get really
| bored we can start coding style flamewars ;-)
|
| Ingo
|
Hi Ingo,
here is a first for x86 tree
- Cyrill -
---
From: Cyrill Gorcunov <gorcunov@...il.com>
Subject: [x86] coding style cleanup for kernel/bootflag.c
This patch eliminates checkpatch.pl complains
on bootflag.c
Signed-off-by: Cyrill Gorcunov <gorcunov@...il.com>
---
arch/x86/kernel/bootflag.c | 40 +++++++++++++++++++++++-----------------
1 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/arch/x86/kernel/bootflag.c b/arch/x86/kernel/bootflag.c
index 0b98605..1697e49 100644
--- a/arch/x86/kernel/bootflag.c
+++ b/arch/x86/kernel/bootflag.c
@@ -24,30 +24,29 @@
int sbf_port __initdata = -1; /* set via acpi_boot_init() */
-
static int __init parity(u8 v)
{
int x = 0;
int i;
-
- for(i=0;i<8;i++)
- {
- x^=(v&1);
- v>>=1;
+
+ for (i = 0; i < 8; i++) {
+ x ^= (v & 1);
+ v >>= 1;
}
+
return x;
}
static void __init sbf_write(u8 v)
{
unsigned long flags;
- if(sbf_port != -1)
- {
+ if (sbf_port != -1) {
v &= ~SBF_PARITY;
- if(!parity(v))
- v|=SBF_PARITY;
+ if (!parity(v))
+ v |= SBF_PARITY;
- printk(KERN_INFO "Simple Boot Flag at 0x%x set to 0x%x\n", sbf_port, v);
+ printk(KERN_INFO "Simple Boot Flag at 0x%x set to 0x%x\n",
+ sbf_port, v);
spin_lock_irqsave(&rtc_lock, flags);
CMOS_WRITE(v, sbf_port);
@@ -59,31 +58,38 @@ static u8 __init sbf_read(void)
{
u8 v;
unsigned long flags;
- if(sbf_port == -1)
+
+ if (sbf_port == -1)
return 0;
+
spin_lock_irqsave(&rtc_lock, flags);
v = CMOS_READ(sbf_port);
spin_unlock_irqrestore(&rtc_lock, flags);
+
return v;
}
static int __init sbf_value_valid(u8 v)
{
- if(v&SBF_RESERVED) /* Reserved bits */
+ if (v & SBF_RESERVED) /* Reserved bits */
return 0;
- if(!parity(v))
+ if (!parity(v))
return 0;
+
return 1;
}
static int __init sbf_init(void)
{
u8 v;
- if(sbf_port == -1)
+
+ if (sbf_port == -1)
return 0;
+
v = sbf_read();
- if(!sbf_value_valid(v))
- printk(KERN_WARNING "Simple Boot Flag value 0x%x read from CMOS RAM was invalid\n",v);
+ if (!sbf_value_valid(v))
+ printk(KERN_WARNING "Simple Boot Flag value 0x%x read from "
+ "CMOS RAM was invalid\n", v);
v &= ~SBF_RESERVED;
v &= ~SBF_BOOTING;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists