Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl
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:
[EMAIL PROTECTED]:/tmp$ size profile.o
textdata bss dec hex filename
3718 144 123874 f22 profile.o
[EMAIL PROTECTED]:/tmp$ size profile.o.after
textdata bss dec hex filename
3693 144 123849 f09 profile.o.after
I'll post the patch for review in a minute.
Thanks.
Ciao,
--
Paolo
http://paolo.ciarrocchi.googlepages.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl
* 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl
On Jan 4, 2008 2:07 PM, Ingo Molnar <[EMAIL PROTECTED]> wrote:
[...]
> > That said, the errors reported by checkpatch.pl are now:
> > [EMAIL PROTECTED]:~/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("X", 0600, root_irq_dir
> > profile.c:594: ERROR: do not use assignment in if condition (+ if
> > (!(entry = create_proc_entry("XXX", 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/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl
* Paolo Ciarrocchi <[EMAIL PROTECTED]> wrote:
> [EMAIL PROTECTED]:~/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:
> [EMAIL PROTECTED]:~/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("X", 0600, root_irq_dir
> profile.c:594: ERROR: do not use assignment in if condition (+ if
> (!(entry = create_proc_entry("XXX", 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl
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:
[EMAIL PROTECTED]:~/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:
[EMAIL PROTECTED]:~/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("X", 0600, root_irq_dir
profile.c:594: ERROR: do not use assignment in if condition (+ if
(!(entry = create_proc_entry("XXX", 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/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl
* 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] This patch to profile.c fixes a few errors reported by checkpatch.pl
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
