[PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl

[PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl

[Paolo Ciarrocchi]

Before:
total: 25 errors, 13 warnings, 602 lines checked

After:
total: 3 errors, 13 warnings, 602 lines checked


Signed-off-by: Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com>
---
Ingo, I'm sending this patch to you since according to git shortlog -e
you are the most active modifier of this file


 kernel/profile.c |   70 +++++++++++++++++++++++++++---------------------------
 1 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/kernel/profile.c b/kernel/profile.c
index 5e95330..c99153b 100644
--- a/kernel/profile.c
+++ b/kernel/profile.c
@@ -52,7 +52,7 @@ static DEFINE_PER_CPU(int, cpu_profile_flip);
 static DEFINE_MUTEX(profile_flip_mutex);
 #endif /* CONFIG_SMP */
 
-static int __init profile_setup(char * str)
+static int __init profile_setup(char *str)
 {
 	static char __initdata schedstr[] = "schedule";
 	static char __initdata sleepstr[] = "sleep";
@@ -104,27 +104,27 @@ __setup("profile=", profile_setup);
 
 void __init profile_init(void)
 {
-	if (!prof_on) 
+	if (!prof_on)
 		return;
- 
+
 	/* only text is profiled */
 	prof_len = (_etext - _stext) >> prof_shift;
 	prof_buffer = alloc_bootmem(prof_len*sizeof(atomic_t));
 }
 
 /* Profile event notifications */
- 
+
 #ifdef CONFIG_PROFILING
- 
+
 static BLOCKING_NOTIFIER_HEAD(task_exit_notifier);
 static ATOMIC_NOTIFIER_HEAD(task_free_notifier);
 static BLOCKING_NOTIFIER_HEAD(munmap_notifier);
- 
-void profile_task_exit(struct task_struct * task)
+
+void profile_task_exit(struct task_struct *task)
 {
 	blocking_notifier_call_chain(&task_exit_notifier, 0, task);
 }
- 
+
 int profile_handoff_task(struct task_struct * task)
 {
 	int ret;
@@ -137,48 +137,48 @@ void profile_munmap(unsigned long addr)
 	blocking_notifier_call_chain(&munmap_notifier, 0, (void *)addr);
 }
 
-int task_handoff_register(struct notifier_block * n)
+int task_handoff_register(struct notifier_block *n)
 {
 	return atomic_notifier_chain_register(&task_free_notifier, n);
 }
 
-int task_handoff_unregister(struct notifier_block * n)
+int task_handoff_unregister(struct notifier_block *n)
 {
 	return atomic_notifier_chain_unregister(&task_free_notifier, n);
 }
 
-int profile_event_register(enum profile_type type, struct notifier_block * n)
+int profile_event_register(enum profile_type type, struct notifier_block *n)
 {
 	int err = -EINVAL;
- 
+
 	switch (type) {
-		case PROFILE_TASK_EXIT:
-			err = blocking_notifier_chain_register(
-					&task_exit_notifier, n);
-			break;
-		case PROFILE_MUNMAP:
-			err = blocking_notifier_chain_register(
-					&munmap_notifier, n);
-			break;
+	case PROFILE_TASK_EXIT:
+		err = blocking_notifier_chain_register(
+				&task_exit_notifier, n);
+		break;
+	case PROFILE_MUNMAP:
+		err = blocking_notifier_chain_register(
+				&munmap_notifier, n);
+		break;
 	}
- 
+
 	return err;
 }
 
- 
-int profile_event_unregister(enum profile_type type, struct notifier_block * n)
+
+int profile_event_unregister(enum profile_type type, struct notifier_block *n)
 {
 	int err = -EINVAL;
- 
+
 	switch (type) {
-		case PROFILE_TASK_EXIT:
-			err = blocking_notifier_chain_unregister(
-					&task_exit_notifier, n);
-			break;
-		case PROFILE_MUNMAP:
-			err = blocking_notifier_chain_unregister(
-					&munmap_notifier, n);
-			break;
+	case PROFILE_TASK_EXIT:
+		err = blocking_notifier_chain_unregister(
+				&task_exit_notifier, n);
+		break;
+	case PROFILE_MUNMAP:
+		err = blocking_notifier_chain_unregister(
+				&munmap_notifier, n);
+		break;
 	}
 
 	return err;
@@ -475,7 +475,7 @@ read_profile(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
 	unsigned long p = *ppos;
 	ssize_t read;
-	char * pnt;
+	char *pnt;
 	unsigned int sample_step = 1 << prof_shift;
 
 	profile_flip_buffers();
@@ -486,12 +486,12 @@ read_profile(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	read = 0;
 
 	while (p < sizeof(unsigned int) && count > 0) {
-		if (put_user(*((char *)(&sample_step)+p),buf))
+		if (put_user(*((char *)(&sample_step)+p), buf))
 			return -EFAULT;
 		buf++; p++; count--; read++;
 	}
 	pnt = (char *)prof_buffer + p - sizeof(atomic_t);
-	if (copy_to_user(buf,(void *)pnt,count))
+	if (copy_to_user(buf, (void *)pnt, count))
 		return -EFAULT;
 	read += count;
 	*ppos += read;
-- 
1.5.4.rc2.17.g257f

Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl

[Jesper Juhl]

On 04/01/2008, Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> wrote:
> Before:
> total: 25 errors, 13 warnings, 602 lines checked
>
> After:
> total: 3 errors, 13 warnings, 602 lines checked
>
>
> Signed-off-by: Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com>

Looks sane to me.

Reviewed-by: Jesper Juhl <jesper.juhl@gmail.com>

-- 
Jesper Juhl <jesper.juhl@gmail.com>
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

Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl

[Ingo Molnar]

* Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> wrote:

> Before:
> total: 25 errors, 13 warnings, 602 lines checked
> 
> After:
> total: 3 errors, 13 warnings, 602 lines checked

thanks, applied. Would you be interested in fixing the other errors and 
warnings too? (Feel free to ask how to resolve certain types of 
warnings. I just took a quick look and all the current checkpatch.pl 
output on profile.c shows genuine style issues.)

	Ingo

Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl

[Paolo Ciarrocchi]

On Jan 4, 2008 9:34 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> wrote:
>
> > Before:
> > total: 25 errors, 13 warnings, 602 lines checked
> >
> > After:
> > total: 3 errors, 13 warnings, 602 lines checked
>
> thanks, applied. Would you be interested in fixing the other errors and
> warnings too? (Feel free to ask how to resolve certain types of
> warnings. I just took a quick look and all the current checkpatch.pl
> output on profile.c shows genuine style issues.)

Yes I am.

First of all I would like to be sure that my usage of checkpatch.pl is corretc,
what I do is the following:

paolo@paolo-desktop:~/linux-2.6/kernel$ ../scripts/checkpatch.pl
--file profile.c

and then I start fixing the errors (so far I didn't start looking at
the warnings).

What I still don't understand are the following options:
 --no-tree    => run without a kernel tree
 --root       => path to the kernel tree root

Should I specify the path to the kernel tree root? If so, why?

That said, the errors reported by checkpatch.pl are now:
paolo@paolo-desktop:~/linux-2.6/kernel$ ../scripts/checkpatch.pl
--terse --file profile.c |grep ERROR
profile.c:128: ERROR: "foo * bar" should be "foo *bar"
I just forgot to fix it, very trivial. Will do in a minute.

profile.c:460: ERROR: do not use assignment in if condition (+  if
(!(entry = create_proc_entry("XXXXXXXXXXXXX", 0600, root_irq_dir))))
profile.c:594: ERROR: do not use assignment in if condition (+  if
(!(entry = create_proc_entry("XXXXXXX", S_IWUSR | S_IRUGO, NULL))))
Here I need an hint ( or an example) about how to fix these two errors :-)

After that, I'll focus on the warnings.

Thanks Ingo!

Ciao,
-- 
Paolo
http://paolo.ciarrocchi.googlepages.com/

Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl

[Ingo Molnar]

* Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> wrote:

> paolo@paolo-desktop:~/linux-2.6/kernel$ ../scripts/checkpatch.pl
> --file profile.c

that's OK.

> What I still don't understand are the following options:
>  --no-tree    => run without a kernel tree
>  --root       => path to the kernel tree root
> 
> Should I specify the path to the kernel tree root? If so, why?

it figures it out itself - if it cannot it will tell you.

> That said, the errors reported by checkpatch.pl are now:
> paolo@paolo-desktop:~/linux-2.6/kernel$ ../scripts/checkpatch.pl
> --terse --file profile.c |grep ERROR
> profile.c:128: ERROR: "foo * bar" should be "foo *bar"
> I just forgot to fix it, very trivial. Will do in a minute.
> 
> profile.c:460: ERROR: do not use assignment in if condition (+  if
> (!(entry = create_proc_entry("XXXXXXXXXXXXX", 0600, root_irq_dir))))
> profile.c:594: ERROR: do not use assignment in if condition (+  if
> (!(entry = create_proc_entry("XXXXXXX", S_IWUSR | S_IRUGO, NULL))))
> Here I need an hint ( or an example) about how to fix these two errors :-)

transform:

  if (!(x = y))

to:

  x = y
  if (!x)

i.e. take the implicit assignment out of the condition. (it's easy to 
mistake it for '==' while reviewing the code and forgetting about the 
assignment's side-effect)

	Ingo

Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl

[Paolo Ciarrocchi]

On Jan 4, 2008 2:07 PM, Ingo Molnar <mingo@elte.hu> wrote:
[...]
> > That said, the errors reported by checkpatch.pl are now:
> > paolo@paolo-desktop:~/linux-2.6/kernel$ ../scripts/checkpatch.pl
> > --terse --file profile.c |grep ERROR
> > profile.c:128: ERROR: "foo * bar" should be "foo *bar"
> > I just forgot to fix it, very trivial. Will do in a minute.
> >
> > profile.c:460: ERROR: do not use assignment in if condition (+  if
> > (!(entry = create_proc_entry("XXXXXXXXXXXXX", 0600, root_irq_dir))))
> > profile.c:594: ERROR: do not use assignment in if condition (+  if
> > (!(entry = create_proc_entry("XXXXXXX", S_IWUSR | S_IRUGO, NULL))))
> > Here I need an hint ( or an example) about how to fix these two errors :-)
>
> transform:
>
>   if (!(x = y))
>
> to:
>
>   x = y
>   if (!x)
>
> i.e. take the implicit assignment out of the condition. (it's easy to
> mistake it for '==' while reviewing the code and forgetting about the
> assignment's side-effect)

OK, thanks.

Is the following correct?

Before:
 if (!(entry = create_proc_entry("prof_cpu_mask", 0600, root_irq_dir)))

After:
entry = create_proc_entry("prof_cpu_mask", 0600, root_irq_dir)
if (!entry)

BTW, how can i compile only the profile.c file?
I would like to verify that my changes (now I'm at total: 2 errors, 1
warnings, 599 lines checked)
doesn't impact on the compiled code?

Thanks.

Ciao,
-- 
Paolo
http://paolo.ciarrocchi.googlepages.com/

Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl

[Ingo Molnar]

* Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> wrote:

> > i.e. take the implicit assignment out of the condition. (it's easy 
> > to mistake it for '==' while reviewing the code and forgetting about 
> > the assignment's side-effect)
> 
> OK, thanks.
> 
> Is the following correct?
> 
> Before:
>  if (!(entry = create_proc_entry("prof_cpu_mask", 0600, root_irq_dir)))
> 
> After:
> entry = create_proc_entry("prof_cpu_mask", 0600, root_irq_dir)
> if (!entry)
> 
> BTW, how can i compile only the profile.c file?

make kernel/profile.o

> I would like to verify that my changes (now I'm at total: 2 errors, 1 
> warnings, 599 lines checked) doesn't impact on the compiled code?

check out:

 http://people.redhat.com/mingo/misc/q-size-obj-compare

which does a size and md5 comparison. (assuming your patch is in a quilt 
queue) But if you reorder symbols (due to the EXPORT_SYMBOL moving) the 
md5 might differ. (but size should still be the same)

	Ingo

Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl

[Paolo Ciarrocchi]

On Jan 4, 2008 2:23 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> wrote:
>
> > > i.e. take the implicit assignment out of the condition. (it's easy
> > > to mistake it for '==' while reviewing the code and forgetting about
> > > the assignment's side-effect)
> >
> > OK, thanks.
> >
> > Is the following correct?
> >
> > Before:
> >  if (!(entry = create_proc_entry("prof_cpu_mask", 0600, root_irq_dir)))
> >
> > After:
> > entry = create_proc_entry("prof_cpu_mask", 0600, root_irq_dir)
> > if (!entry)
> >
> > BTW, how can i compile only the profile.c file?
>
> make kernel/profile.o

Thanks.

> > I would like to verify that my changes (now I'm at total: 2 errors, 1
> > warnings, 599 lines checked) doesn't impact on the compiled code?
>
> check out:
>
>  http://people.redhat.com/mingo/misc/q-size-obj-compare
>
> which does a size and md5 comparison. (assuming your patch is in a quilt
> queue) But if you reorder symbols (due to the EXPORT_SYMBOL moving) the
> md5 might differ. (but size should still be the same)

I'm not using quilt but git.

Unfortunatelly size changed:
paolo@paolo-desktop:/tmp$ size profile.o
   text    data     bss     dec     hex filename
   3718     144      12    3874     f22 profile.o
paolo@paolo-desktop:/tmp$ size profile.o.after
   text    data     bss     dec     hex filename
   3693     144      12    3849     f09 profile.o.after

I'll post the patch for review in a minute.

Thanks.

Ciao,
-- 
Paolo
http://paolo.ciarrocchi.googlepages.com/