Before:
total: 25 errors, 13 warnings, 602 lines checked
After:
total: 3 errors, 13 warnings, 602 lines checked
Signed-off-by: Paolo Ciarrocchi <[email protected]>
---
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
On 04/01/2008, Paolo Ciarrocchi <[email protected]> wrote:
> Before:
> total: 25 errors, 13 warnings, 602 lines checked
>
> After:
> total: 3 errors, 13 warnings, 602 lines checked
>
>
> Signed-off-by: Paolo Ciarrocchi <[email protected]>
Looks sane to me.
Reviewed-by: Jesper Juhl <[email protected]>
--
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
* Paolo Ciarrocchi <[email protected]> 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
On Jan 4, 2008 9:34 AM, Ingo Molnar <[email protected]> wrote:
>
> * Paolo Ciarrocchi <[email protected]> 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/
* Paolo Ciarrocchi <[email protected]> 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
On Jan 4, 2008 2:07 PM, Ingo Molnar <[email protected]> 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/
* Paolo Ciarrocchi <[email protected]> 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
On Jan 4, 2008 2:23 PM, Ingo Molnar <[email protected]> wrote:
>
> * Paolo Ciarrocchi <[email protected]> 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/