Fix plenty of coding style errors
Checkpatch now reports:
total: 2 errors, 18 warnings, 526 lines checked
Signed-off-by: Paolo Ciarrocchi <[email protected]>
---
arch/x86/ia32/ia32_aout.c | 100 ++++++++++++++++++++++-----------------------
1 files changed, 49 insertions(+), 51 deletions(-)
diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
index f82e1a9..65dc50a 100644
--- a/arch/x86/ia32/ia32_aout.c
+++ b/arch/x86/ia32/ia32_aout.c
@@ -36,8 +36,8 @@
#undef WARN_OLD
#undef CORE_DUMP /* probably broken */
-static int load_aout_binary(struct linux_binprm *, struct pt_regs * regs);
-static int load_aout_library(struct file*);
+static int load_aout_binary(struct linux_binprm *, struct pt_regs *regs);
+static int load_aout_library(struct file *);
#ifdef CORE_DUMP
static int aout_core_dump(long signr, struct pt_regs *regs, struct file *file, unsigned long limit);
@@ -45,9 +45,9 @@ static int aout_core_dump(long signr, struct pt_regs *regs, struct file *file, u
/*
* fill in the user structure for a core dump..
*/
-static void dump_thread32(struct pt_regs * regs, struct user32 * dump)
+static void dump_thread32(struct pt_regs *regs, struct user32 *dump)
{
- u32 fs,gs;
+ u32 fs, gs;
/* changed the size calculations - should hopefully work better. lbt */
dump->magic = CMAGIC;
@@ -57,14 +57,14 @@ static void dump_thread32(struct pt_regs * regs, struct user32 * dump)
dump->u_dsize = ((unsigned long) (current->mm->brk + (PAGE_SIZE-1))) >> PAGE_SHIFT;
dump->u_dsize -= dump->u_tsize;
dump->u_ssize = 0;
- dump->u_debugreg[0] = current->thread.debugreg0;
- dump->u_debugreg[1] = current->thread.debugreg1;
- dump->u_debugreg[2] = current->thread.debugreg2;
- dump->u_debugreg[3] = current->thread.debugreg3;
- dump->u_debugreg[4] = 0;
- dump->u_debugreg[5] = 0;
- dump->u_debugreg[6] = current->thread.debugreg6;
- dump->u_debugreg[7] = current->thread.debugreg7;
+ dump->u_debugreg[0] = current->thread.debugreg0;
+ dump->u_debugreg[1] = current->thread.debugreg1;
+ dump->u_debugreg[2] = current->thread.debugreg2;
+ dump->u_debugreg[3] = current->thread.debugreg3;
+ dump->u_debugreg[4] = 0;
+ dump->u_debugreg[5] = 0;
+ dump->u_debugreg[6] = current->thread.debugreg6;
+ dump->u_debugreg[7] = current->thread.debugreg7;
if (dump->start_stack < 0xc0000000)
dump->u_ssize = ((unsigned long) (0xc0000000 - dump->start_stack)) >> PAGE_SHIFT;
@@ -79,7 +79,7 @@ static void dump_thread32(struct pt_regs * regs, struct user32 * dump)
dump->regs.ds = current->thread.ds;
dump->regs.es = current->thread.es;
asm("movl %%fs,%0" : "=r" (fs)); dump->regs.fs = fs;
- asm("movl %%gs,%0" : "=r" (gs)); dump->regs.gs = gs;
+ asm("movl %%gs,%0" : "=r" (gs)); dump->regs.gs = gs;
dump->regs.orig_eax = regs->orig_rax;
dump->regs.eip = regs->rip;
dump->regs.cs = regs->cs;
@@ -90,7 +90,7 @@ static void dump_thread32(struct pt_regs * regs, struct user32 * dump)
#if 1 /* FIXME */
dump->u_fpvalid = 0;
#else
- dump->u_fpvalid = dump_fpu (regs, &dump->i387);
+ dump->u_fpvalid = dump_fpu(regs, &dump->i387);
#endif
}
@@ -134,8 +134,8 @@ static int dump_write(struct file *file, const void *addr, int nr)
#define DUMP_SEEK(offset) \
if (file->f_op->llseek) { \
- if (file->f_op->llseek(file,(offset),0) != (offset)) \
- goto end_coredump; \
+ if (file->f_op->llseek(file, (offset), 0) != (offset)) \
+ goto end_coredump; \
} else file->f_pos = (offset)
/*
@@ -161,7 +161,7 @@ static int aout_core_dump(long signr, struct pt_regs *regs, struct file *file, u
set_fs(KERNEL_DS);
has_dumped = 1;
current->flags |= PF_DUMPCORE;
- strncpy(dump.u_comm, current->comm, sizeof(current->comm));
+ strncpy(dump.u_comm, current->comm, sizeof(current->comm));
dump.u_ar0 = (u32)(((unsigned long)(&dump.regs)) - ((unsigned long)(&dump)));
dump.signal = signr;
dump_thread32(regs, &dump);
@@ -184,7 +184,7 @@ static int aout_core_dump(long signr, struct pt_regs *regs, struct file *file, u
set_fs(KERNEL_DS);
/* struct user */
- DUMP_WRITE(&dump,sizeof(dump));
+ DUMP_WRITE(&dump, sizeof(dump));
/* Now dump all of the user data. Include malloced stuff as well */
DUMP_SEEK(PAGE_SIZE);
/* now we start writing out the user space info */
@@ -193,17 +193,17 @@ static int aout_core_dump(long signr, struct pt_regs *regs, struct file *file, u
if (dump.u_dsize != 0) {
dump_start = START_DATA(dump);
dump_size = dump.u_dsize << PAGE_SHIFT;
- DUMP_WRITE(dump_start,dump_size);
+ DUMP_WRITE(dump_start, dump_size);
}
/* Now prepare to dump the stack area */
if (dump.u_ssize != 0) {
dump_start = START_STACK(dump);
dump_size = dump.u_ssize << PAGE_SHIFT;
- DUMP_WRITE(dump_start,dump_size);
+ DUMP_WRITE(dump_start, dump_size);
}
/* Finally dump the task struct. Not be used by gdb, but could be useful */
set_fs(KERNEL_DS);
- DUMP_WRITE(current,sizeof(*current));
+ DUMP_WRITE(current, sizeof(*current));
end_coredump:
set_fs(fs);
return has_dumped;
@@ -228,24 +228,24 @@ static u32 __user *create_aout_tables(char __user *p, struct linux_binprm *bprm)
envp = sp;
sp -= argc+1;
argv = sp;
- put_user((unsigned long) envp,--sp);
- put_user((unsigned long) argv,--sp);
- put_user(argc,--sp);
+ put_user((unsigned long) envp, --sp);
+ put_user((unsigned long) argv, --sp);
+ put_user(argc, --sp);
current->mm->arg_start = (unsigned long) p;
- while (argc-->0) {
+ while (argc-- > 0) {
char c;
- put_user((u32)(unsigned long)p,argv++);
+ put_user((u32)(unsigned long)p, argv++);
do {
- get_user(c,p++);
+ get_user(c, p++);
} while (c);
}
put_user(0, argv);
current->mm->arg_end = current->mm->env_start = (unsigned long) p;
- while (envc-->0) {
+ while (envc-- > 0) {
char c;
- put_user((u32)(unsigned long)p,envp++);
+ put_user((u32)(unsigned long)p, envp++);
do {
- get_user(c,p++);
+ get_user(c, p++);
} while (c);
}
put_user(0, envp);
@@ -258,7 +258,7 @@ static u32 __user *create_aout_tables(char __user *p, struct linux_binprm *bprm)
* libraries. There is no binary dependent code anywhere else.
*/
-static int load_aout_binary(struct linux_binprm * bprm, struct pt_regs * regs)
+static int load_aout_binary(struct linux_binprm *bprm, struct pt_regs *regs)
{
struct exec ex;
unsigned long error;
@@ -291,13 +291,13 @@ static int load_aout_binary(struct linux_binprm * bprm, struct pt_regs * regs)
if (retval)
return retval;
- regs->cs = __USER32_CS;
+ regs->cs = __USER32_CS;
regs->r8 = regs->r9 = regs->r10 = regs->r11 = regs->r12 =
regs->r13 = regs->r14 = regs->r15 = 0;
/* OK, This is the point of no return */
set_personality(PER_LINUX);
- set_thread_flag(TIF_IA32);
+ set_thread_flag(TIF_IA32);
clear_thread_flag(TIF_ABI_PENDING);
current->mm->end_code = ex.a_text +
@@ -311,7 +311,7 @@ static int load_aout_binary(struct linux_binprm * bprm, struct pt_regs * regs)
current->mm->mmap = NULL;
compute_creds(bprm);
- current->flags &= ~PF_FORKNOEXEC;
+ current->flags &= ~PF_FORKNOEXEC;
if (N_MAGIC(ex) == OMAGIC) {
unsigned long text_addr, map_size;
@@ -338,29 +338,27 @@ static int load_aout_binary(struct linux_binprm * bprm, struct pt_regs * regs)
send_sig(SIGKILL, current, 0);
return error;
}
-
+
flush_icache_range(text_addr, text_addr+ex.a_text+ex.a_data);
} else {
#ifdef WARN_OLD
static unsigned long error_time, error_time2;
if ((ex.a_text & 0xfff || ex.a_data & 0xfff) &&
- (N_MAGIC(ex) != NMAGIC) && (jiffies-error_time2) > 5*HZ)
- {
+ (N_MAGIC(ex) != NMAGIC) && (jiffies-error_time2) > 5*HZ) {
printk(KERN_NOTICE "executable not page aligned\n");
error_time2 = jiffies;
}
if ((fd_offset & ~PAGE_MASK) != 0 &&
- (jiffies-error_time) > 5*HZ)
- {
- printk(KERN_WARNING
+ (jiffies-error_time) > 5*HZ) {
+ printk(KERN_WARNING
"fd_offset is not page aligned. Please convert program: %s\n",
bprm->file->f_path.dentry->d_name.name);
error_time = jiffies;
}
#endif
- if (!bprm->file->f_op->mmap||((fd_offset & ~PAGE_MASK) != 0)) {
+ if (!bprm->file->f_op->mmap || ((fd_offset & ~PAGE_MASK) != 0)) {
loff_t pos = fd_offset;
down_write(¤t->mm->mmap_sem);
do_brk(N_TXTADDR(ex), ex.a_text+ex.a_data);
@@ -387,7 +385,7 @@ static int load_aout_binary(struct linux_binprm * bprm, struct pt_regs * regs)
}
down_write(¤t->mm->mmap_sem);
- error = do_mmap(bprm->file, N_DATADDR(ex), ex.a_data,
+ error = do_mmap(bprm->file, N_DATADDR(ex), ex.a_data,
PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE | MAP_32BIT,
fd_offset + ex.a_text);
@@ -403,9 +401,9 @@ beyond_if:
set_brk(current->mm->start_brk, current->mm->brk);
retval = setup_arg_pages(bprm, IA32_STACK_TOP, EXSTACK_DEFAULT);
- if (retval < 0) {
- /* Someone check-me: is this error path enough? */
- send_sig(SIGKILL, current, 0);
+ if (retval < 0) {
+ /* Someone check-me: is this error path enough? */
+ send_sig(SIGKILL, current, 0);
return retval;
}
@@ -414,7 +412,7 @@ beyond_if:
/* start thread */
asm volatile("movl %0,%%fs" :: "r" (0)); \
asm volatile("movl %0,%%es; movl %0,%%ds": :"r" (__USER32_DS));
- load_gs_index(0);
+ load_gs_index(0);
(regs)->rip = ex.a_entry;
(regs)->rsp = current->mm->start_stack;
(regs)->eflags = 0x200;
@@ -434,7 +432,7 @@ beyond_if:
static int load_aout_library(struct file *file)
{
- struct inode * inode;
+ struct inode *inode;
unsigned long bss, start_addr, len;
unsigned long error;
int retval;
@@ -467,9 +465,9 @@ static int load_aout_library(struct file *file)
#ifdef WARN_OLD
static unsigned long error_time;
- if ((jiffies-error_time) > 5*HZ)
- {
- printk(KERN_WARNING
+ if ((jiffies-error_time) > 5*HZ) {
+
+ printk(KERN_WARNING
"N_TXTOFF is not page aligned. Please convert library: %s\n",
file->f_path.dentry->d_name.name);
error_time = jiffies;
@@ -478,7 +476,7 @@ static int load_aout_library(struct file *file)
down_write(¤t->mm->mmap_sem);
do_brk(start_addr, ex.a_text + ex.a_data + ex.a_bss);
up_write(¤t->mm->mmap_sem);
-
+
file->f_op->read(file, (char __user *)start_addr,
ex.a_text + ex.a_data, &pos);
flush_icache_range((unsigned long) start_addr,
--
1.5.4.rc2.17.g257f
> - if ((jiffies-error_time) > 5*HZ)
> - {
> - printk(KERN_WARNING
> + if ((jiffies-error_time) > 5*HZ) {
Should we expect second wave of this crap when you will suddenly realize
that it would be even nicer to write:
if (jiffies - error_time > 5 * HZ) {
Alexey "?" Dobriyan
On Tue, 8 Jan 2008 20:32:33 +0100
Paolo Ciarrocchi <[email protected]> wrote:
> Fix plenty of coding style errors
Most of these kernel changes would probably get in the way of
real development, making patches reject that would otherwise
apply.
You did find one possible bug, though:
> @@ -467,9 +465,9 @@ static int load_aout_library(struct file *file)
>
> #ifdef WARN_OLD
> static unsigned long error_time;
> - if ((jiffies-error_time) > 5*HZ)
> - {
> - printk(KERN_WARNING
> + if ((jiffies-error_time) > 5*HZ) {
> +
> + printk(KERN_WARNING
> "N_TXTOFF is not page aligned. Please convert library: %s\n",
> file->f_path.dentry->d_name.name);
> error_time = jiffies;
You may want to look into the time_after() macro and make sure
it is used here.
--
All rights reversed.
On Tue, Jan 08, 2008 at 03:04:39PM -0500, Rik van Riel wrote:
> On Tue, 8 Jan 2008 20:32:33 +0100
> Paolo Ciarrocchi <[email protected]> wrote:
>
> > Fix plenty of coding style errors
>
> Most of these kernel changes would probably get in the way of
> real development, making patches reject that would otherwise
> apply.
>
> You did find one possible bug, though:
>
> > @@ -467,9 +465,9 @@ static int load_aout_library(struct file *file)
> >
> > #ifdef WARN_OLD
> > static unsigned long error_time;
> > - if ((jiffies-error_time) > 5*HZ)
> > - {
> > - printk(KERN_WARNING
> > + if ((jiffies-error_time) > 5*HZ) {
> > +
> > + printk(KERN_WARNING
> > "N_TXTOFF is not page aligned. Please convert library: %s\n",
> > file->f_path.dentry->d_name.name);
> > error_time = jiffies;
>
> You may want to look into the time_after() macro and make sure
> it is used here.
It is already fixed in the x86 tree in the mm branch.
So this part would conflict with ongoing development effort...
Sam
* Rik van Riel <[email protected]> wrote:
> On Tue, 8 Jan 2008 20:32:33 +0100
> Paolo Ciarrocchi <[email protected]> wrote:
>
> > Fix plenty of coding style errors
>
> Most of these kernel changes would probably get in the way of real
> development, making patches reject that would otherwise apply.
I'm curious, in what way would they interfere?
Firstly, anyone with a forked kernel with outstanding patches that are
not in x86.git only has themselves to blame. We want to actively
discourage forking and sitting on patches too long.
Secondly, when there _is_ some non-trivial interaction with reasonably
recently-developed patches, the solution is simple and straightforward
we simply undo the relevant portions of the cleanup, apply the
functional patch and later on apply the (still relevant) cleanup patches
to around the functional patch.
Since all new x86.git patches are checkpatch.pl clean, the modified
portions need no cleanups anymore - only unmodified portions.
How many times did we have to do this in x86.git? Once or twice - out of
100+ cleanup patches.
In reality, rarely do cleanup patches interfere. They have two positive
effects besides the obvious readability, debuggability and
maintainability advantages:
- they _do_ cause people to come out of their distro-patched
fork-woodwork and submit their "development" patches (which were "in
the works" for ... years).
- the cleanups make future development _easier_, because it's easier to
develop on a clean codebase.
and because the number of future patches is infinitely larger than the
number of still pending but not submitted development patches, we
strongly favor cleanups.
so in the end it all works out fine.
Ingo
> > Most of these kernel changes would probably get in the way of real
> > development, making patches reject that would otherwise apply.
>
> I'm curious, in what way would they interfere?
Developer A work one some complicated stuff in foo.c which is
not yet -mm fooder.
Developer B submits and have applied a massive cleanup to some of the
files touced by Developer A's patch.
Developer A now needs to fix up his stuff.
Reminder: Not everyone writes their stuff in 48 hours before it
is lkml ready.
>
> Firstly, anyone with a forked kernel with outstanding patches that are
> not in x86.git only has themselves to blame. We want to actively
> discourage forking and sitting on patches too long.
Curious - what is the purpose of the x86.git tree these days?
Sam
* Sam Ravnborg <[email protected]> wrote:
> > > Most of these kernel changes would probably get in the way of real
> > > development, making patches reject that would otherwise apply.
> >
> > I'm curious, in what way would they interfere?
>
> Developer A work one some complicated stuff in foo.c which is not yet
> -mm fooder.
>
> Developer B submits and have applied a massive cleanup to some of the
> files touced by Developer A's patch.
>
> Developer A now needs to fix up his stuff.
Solution: Developer A does a trivial patch -R for the changes that
generate rejects. Cleanups are NOPs and they are almost infinitely
splittable. You can apply and unapply them chunk by chunk, almost
always.
and we actually have first-hand experience with the effects of
largescale cleanups:
> > How many times did we have to do this in x86.git? Once or twice -
> > out of 100+ cleanup patches.
i've never seen cleanups truly interfere with development work, in fact
i mostly saw the positive effects of them. They are easily undone and
easily redone. But they do keep developers honest (no lame "oh, i'll
clean this up after i do feature X, Y and Z") and they do keep newbies
involved. The Linux kernel does have a fundamental "we are too hostile
towards newbies" problem. It's also fundamentally Linuxish: nobody
really "owns" the code in an exclusive fashion. If you dont keep it
clean, someone else will clean it up for you.
and even if there are patch conflicts, it can all be done intelligently
and trivially. Mail us: "please do not clean up pgtable.h because i'm
working on unifying it and will do the cleanup after that" and we dont
clean it up.
But broad statements of "cleanups hinder development work" are just
plain _WRONG_. Cleanups are good by default - full stop. There are
exceptions, and we all recognize them when we see them.
> > Firstly, anyone with a forked kernel with outstanding patches that
> > are not in x86.git only has themselves to blame. We want to actively
> > discourage forking and sitting on patches too long.
>
> Curious - what is the purpose of the x86.git tree these days?
what do want to imply by 'these days'?
Ingo
On 01/08/2008 11:17 PM, Sam Ravnborg wrote:
>>> Most of these kernel changes would probably get in the way of real
>>> development, making patches reject that would otherwise apply.
>> I'm curious, in what way would they interfere?
>
> Developer A work one some complicated stuff in foo.c which is
> not yet -mm fooder.
>
> Developer B submits and have applied a massive cleanup to some of the
> files touced by Developer A's patch.
>
> Developer A now needs to fix up his stuff.
Ok, to be honest, how often is this a problem?
And then, how hard is it to rebase the patch?
And if it is a problem, then you can still drop a message, such as don't do
this, I have a big patch here.
>
> > > Firstly, anyone with a forked kernel with outstanding patches that
> > > are not in x86.git only has themselves to blame. We want to actively
> > > discourage forking and sitting on patches too long.
> >
> > Curious - what is the purpose of the x86.git tree these days?
>
> what do want to imply by 'these days'?
I wondered when you wrote "anyone with a forked kernel with outstanding
patches that are not in x86.git" if this was only x86 specific patches
or more than that. I could have a slev of patches in the works
for parts that are no x86 specific (which I unfortunately do not have).
Sam
* Sam Ravnborg <[email protected]> wrote:
> > > > Firstly, anyone with a forked kernel with outstanding patches
> > > > that are not in x86.git only has themselves to blame. We want to
> > > > actively discourage forking and sitting on patches too long.
> > >
> > > Curious - what is the purpose of the x86.git tree these days?
> >
> > what do want to imply by 'these days'?
>
> I wondered when you wrote "anyone with a forked kernel with
> outstanding patches that are not in x86.git" if this was only x86
> specific patches or more than that. I could have a slev of patches in
> the works for parts that are no x86 specific (which I unfortunately do
> not have).
ah, i now understand what you mean. The stuff in -mm that touches
arch/x86 is for actively maintained areas which are generally quite
clean.
So this is not a problem in practice - massively unclean areas of code,
which are the primary target for cleanups, are not actively developed. [
perhaps because there's some level of correlation between unclean code
and lack of developer interest :-/ ]
Ingo