[PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Joe Perches]

Seems to be a typo.

Signed-off-by: Joe Perches <joe@perches.com>

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 0d949c5..d0bf4eb 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1605,7 +1605,7 @@ static struct ctl_table debug_table[] = {
 		.data		= &show_unhandled_signals,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec
+		.proc_handler	= &proc_dointvec,
 	},
 #endif
 	{ .ctl_name = 0 }

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Américo Wang]

On Sat, Nov 14, 2009 at 05:52:05PM -0800, Joe Perches wrote:
>Seems to be a typo.
>

Well, in this case, proc_dointvec == &proc_dointvec,
both of them are the same function pointer.

But yes, &proc_dointvec looks more readable.

>Signed-off-by: Joe Perches <joe@perches.com>

Acked-by: WANG Cong <xiyou.wangcong@gmail.com>

>
>diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>index 0d949c5..d0bf4eb 100644
>--- a/kernel/sysctl.c
>+++ b/kernel/sysctl.c
>@@ -1605,7 +1605,7 @@ static struct ctl_table debug_table[] = {
> 		.data		= &show_unhandled_signals,
> 		.maxlen		= sizeof(int),
> 		.mode		= 0644,
>-		.proc_handler	= proc_dointvec
>+		.proc_handler	= &proc_dointvec,
> 	},
> #endif
> 	{ .ctl_name = 0 }
>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

-- 
Live like a child, think like the god.
 

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Ingo Molnar]

* Am??rico Wang <xiyou.wangcong@gmail.com> wrote:

> On Sat, Nov 14, 2009 at 05:52:05PM -0800, Joe Perches wrote:
> >Seems to be a typo.
> >
> 
> Well, in this case, proc_dointvec == &proc_dointvec,
> both of them are the same function pointer.
> 
> But yes, &proc_dointvec looks more readable.
> 
> >Signed-off-by: Joe Perches <joe@perches.com>
> 
> Acked-by: WANG Cong <xiyou.wangcong@gmail.com>

(Cc:-ed Eric who is running the sysctl tree these days)

Almost everywhere in the kernel we use the shorter version, so all of 
sysctl.c should eventually change to that variant.

	Ingo

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Joe Perches]

On Sun, 2009-11-15 at 09:11 +0100, Ingo Molnar wrote:
> * Am??rico Wang <xiyou.wangcong@gmail.com> wrote:
> > On Sat, Nov 14, 2009 at 05:52:05PM -0800, Joe Perches wrote:
> > >Seems to be a typo.
> > Acked-by: WANG Cong <xiyou.wangcong@gmail.com>
> (Cc:-ed Eric who is running the sysctl tree these days)
> Almost everywhere in the kernel we use the shorter version, so all of 
> sysctl.c should eventually change to that variant.

It's closer to 50/50, but it's 1 vs 133 in that file.

$ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*&\s*\w+' * | wc -l
339

$ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*[^&]\s*\w+' * | wc -l
432

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Ingo Molnar]

* Joe Perches <joe@perches.com> wrote:

> On Sun, 2009-11-15 at 09:11 +0100, Ingo Molnar wrote:
> > * Am??rico Wang <xiyou.wangcong@gmail.com> wrote:
> > > On Sat, Nov 14, 2009 at 05:52:05PM -0800, Joe Perches wrote:
> > > >Seems to be a typo.
> > > Acked-by: WANG Cong <xiyou.wangcong@gmail.com>
> > (Cc:-ed Eric who is running the sysctl tree these days)
> > Almost everywhere in the kernel we use the shorter version, so all of 
> > sysctl.c should eventually change to that variant.
> 
> It's closer to 50/50, but it's 1 vs 133 in that file.
> 
> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*&\s*\w+' * | wc -l
> 339
> 
> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*[^&]\s*\w+' * | wc -l
> 432

I did not mean this specific initialization method of proc_handler, i 
meant pointers to functions in general.

	Ingo

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Eric W. Biederman]

Ingo Molnar <mingo@elte.hu> writes:

> * Joe Perches <joe@perches.com> wrote:
>
>> On Sun, 2009-11-15 at 09:11 +0100, Ingo Molnar wrote:
>> > * Am??rico Wang <xiyou.wangcong@gmail.com> wrote:
>> > > On Sat, Nov 14, 2009 at 05:52:05PM -0800, Joe Perches wrote:
>> > > >Seems to be a typo.
>> > > Acked-by: WANG Cong <xiyou.wangcong@gmail.com>
>> > (Cc:-ed Eric who is running the sysctl tree these days)
>> > Almost everywhere in the kernel we use the shorter version, so all of 
>> > sysctl.c should eventually change to that variant.
>> 
>> It's closer to 50/50, but it's 1 vs 133 in that file.
>> 
>> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*&\s*\w+' * | wc -l
>> 339
>> 
>> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*[^&]\s*\w+' * | wc -l
>> 432
>
> I did not mean this specific initialization method of proc_handler, i 
> meant pointers to functions in general.


There was an argument put forward by Alexy (I think) a while ago.  That
argued for the form without the address of operator.

The reason being that without it you can do:
#define proc_dointvec NULL

in a header when sysctl support it compiled out.  Using address of
you wind up with stub functions in sysctl.c to handle the case when
sysctl is compiled out.

It isn't a strong case but since not using & is also shorter and
as Ingo pointed out more common I think no & wins.

Eric

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Ingo Molnar]

* Eric W. Biederman <ebiederm@xmission.com> wrote:

> Ingo Molnar <mingo@elte.hu> writes:
> 
> > * Joe Perches <joe@perches.com> wrote:
> >
> >> On Sun, 2009-11-15 at 09:11 +0100, Ingo Molnar wrote:
> >> > * Am??rico Wang <xiyou.wangcong@gmail.com> wrote:
> >> > > On Sat, Nov 14, 2009 at 05:52:05PM -0800, Joe Perches wrote:
> >> > > >Seems to be a typo.
> >> > > Acked-by: WANG Cong <xiyou.wangcong@gmail.com>
> >> > (Cc:-ed Eric who is running the sysctl tree these days)
> >> > Almost everywhere in the kernel we use the shorter version, so all of 
> >> > sysctl.c should eventually change to that variant.
> >> 
> >> It's closer to 50/50, but it's 1 vs 133 in that file.
> >> 
> >> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*&\s*\w+' * | wc -l
> >> 339
> >> 
> >> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*[^&]\s*\w+' * | wc -l
> >> 432
> >
> > I did not mean this specific initialization method of proc_handler, i 
> > meant pointers to functions in general.
> 
> 
> There was an argument put forward by Alexy (I think) a while ago.  
> That argued for the form without the address of operator.
> 
> The reason being that without it you can do:
> #define proc_dointvec NULL
> 
> in a header when sysctl support it compiled out.  Using address of
> you wind up with stub functions in sysctl.c to handle the case when
> sysctl is compiled out.
> 
> It isn't a strong case but since not using & is also shorter and as 
> Ingo pointed out more common I think no & wins.

I can think of another reason as well: the & operator can be dangerous 
if code is changed from functions to function pointers.

The short form:

  val = do_my_func;

will work just fine if 'my_func' is changed to a function pointer, as it 
will evaluate to the value of the function pointer - i.e. the address of 
the function.

The longer form:

  val = &do_my_func;

might break in a subtle way, because it will now become the address of 
the function pointer - not the function address.

Combined the shortness, the NULL init, the function pointer invariance, 
plus existing in-kernel practice all suggest that the short form should 
be used.

( i didnt want to turn this small issue into a long argument - it's just
  that the code was going in the wrong direction. )

	Ingo

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Eric W. Biederman]

Ingo Molnar <mingo@elte.hu> writes:

> * Eric W. Biederman <ebiederm@xmission.com> wrote:
>
>> Ingo Molnar <mingo@elte.hu> writes:
>> 
>> > * Joe Perches <joe@perches.com> wrote:
>> >
>> >> On Sun, 2009-11-15 at 09:11 +0100, Ingo Molnar wrote:
>> >> > * Am??rico Wang <xiyou.wangcong@gmail.com> wrote:
>> >> > > On Sat, Nov 14, 2009 at 05:52:05PM -0800, Joe Perches wrote:
>> >> > > >Seems to be a typo.
>> >> > > Acked-by: WANG Cong <xiyou.wangcong@gmail.com>
>> >> > (Cc:-ed Eric who is running the sysctl tree these days)
>> >> > Almost everywhere in the kernel we use the shorter version, so all of 
>> >> > sysctl.c should eventually change to that variant.
>> >> 
>> >> It's closer to 50/50, but it's 1 vs 133 in that file.
>> >> 
>> >> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*&\s*\w+' * | wc -l
>> >> 339
>> >> 
>> >> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*[^&]\s*\w+' * | wc -l
>> >> 432
>> >
>> > I did not mean this specific initialization method of proc_handler, i 
>> > meant pointers to functions in general.
>> 
>> 
>> There was an argument put forward by Alexy (I think) a while ago.  
>> That argued for the form without the address of operator.
>> 
>> The reason being that without it you can do:
>> #define proc_dointvec NULL
>> 
>> in a header when sysctl support it compiled out.  Using address of
>> you wind up with stub functions in sysctl.c to handle the case when
>> sysctl is compiled out.
>> 
>> It isn't a strong case but since not using & is also shorter and as 
>> Ingo pointed out more common I think no & wins.
>
> I can think of another reason as well: the & operator can be dangerous 
> if code is changed from functions to function pointers.
>
> The short form:
>
>   val = do_my_func;
>
> will work just fine if 'my_func' is changed to a function pointer, as it 
> will evaluate to the value of the function pointer - i.e. the address of 
> the function.
>
> The longer form:
>
>   val = &do_my_func;
>
> might break in a subtle way, because it will now become the address of 
> the function pointer - not the function address.
>
> Combined the shortness, the NULL init, the function pointer invariance, 
> plus existing in-kernel practice all suggest that the short form should 
> be used.
>
> ( i didnt want to turn this small issue into a long argument - it's just
>   that the code was going in the wrong direction. )

No problem here.  I'm still working through how to keep my tree from
conflicting with the net tree, big sweeping tend to have that problem,
but if someone wants to generate the & removal patches (against my
tree) and send them to me I will be happy to host them.  I am already
touching practically every sysctl table in the tree.

Eric

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Ingo Molnar]

* Eric W. Biederman <ebiederm@xmission.com> wrote:

> Ingo Molnar <mingo@elte.hu> writes:
> 
> > * Eric W. Biederman <ebiederm@xmission.com> wrote:
> >
> >> Ingo Molnar <mingo@elte.hu> writes:
> >> 
> >> > * Joe Perches <joe@perches.com> wrote:
> >> >
> >> >> On Sun, 2009-11-15 at 09:11 +0100, Ingo Molnar wrote:
> >> >> > * Am??rico Wang <xiyou.wangcong@gmail.com> wrote:
> >> >> > > On Sat, Nov 14, 2009 at 05:52:05PM -0800, Joe Perches wrote:
> >> >> > > >Seems to be a typo.
> >> >> > > Acked-by: WANG Cong <xiyou.wangcong@gmail.com>
> >> >> > (Cc:-ed Eric who is running the sysctl tree these days)
> >> >> > Almost everywhere in the kernel we use the shorter version, so all of 
> >> >> > sysctl.c should eventually change to that variant.
> >> >> 
> >> >> It's closer to 50/50, but it's 1 vs 133 in that file.
> >> >> 
> >> >> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*&\s*\w+' * | wc -l
> >> >> 339
> >> >> 
> >> >> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*[^&]\s*\w+' * | wc -l
> >> >> 432
> >> >
> >> > I did not mean this specific initialization method of proc_handler, i 
> >> > meant pointers to functions in general.
> >> 
> >> 
> >> There was an argument put forward by Alexy (I think) a while ago.  
> >> That argued for the form without the address of operator.
> >> 
> >> The reason being that without it you can do:
> >> #define proc_dointvec NULL
> >> 
> >> in a header when sysctl support it compiled out.  Using address of
> >> you wind up with stub functions in sysctl.c to handle the case when
> >> sysctl is compiled out.
> >> 
> >> It isn't a strong case but since not using & is also shorter and as 
> >> Ingo pointed out more common I think no & wins.
> >
> > I can think of another reason as well: the & operator can be dangerous 
> > if code is changed from functions to function pointers.
> >
> > The short form:
> >
> >   val = do_my_func;
> >
> > will work just fine if 'my_func' is changed to a function pointer, as it 
> > will evaluate to the value of the function pointer - i.e. the address of 
> > the function.
> >
> > The longer form:
> >
> >   val = &do_my_func;
> >
> > might break in a subtle way, because it will now become the address of 
> > the function pointer - not the function address.
> >
> > Combined the shortness, the NULL init, the function pointer invariance, 
> > plus existing in-kernel practice all suggest that the short form should 
> > be used.
> >
> > ( i didnt want to turn this small issue into a long argument - it's just
> >   that the code was going in the wrong direction. )
> 
> No problem here.  I'm still working through how to keep my tree from 
> conflicting with the net tree, big sweeping tend to have that problem, 
> but if someone wants to generate the & removal patches (against my 
> tree) and send them to me I will be happy to host them.  I am already 
> touching practically every sysctl table in the tree.

The preferred flow is for you to just work against Linus's latest tree - 
and everyone will deal with the (mostly trivial) conflicts when they 
happen. Linus prefers to resolve conflicts himself when he pulls, 
because people mixing their trees (such as you basing on net-next for 
example) leads to various dependency problems.

	Ingo

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Eric W. Biederman]

Ingo Molnar <mingo@elte.hu> writes:

> The preferred flow is for you to just work against Linus's latest tree - 
> and everyone will deal with the (mostly trivial) conflicts when they 
> happen. Linus prefers to resolve conflicts himself when he pulls, 
> because people mixing their trees (such as you basing on net-next for 
> example) leads to various dependency problems.

Sounds right.

I'm still getting up the courage to conflict.  I was wondering if I
could cherry pick a patch or two to avoid those.

Right now in the net tree there is one new sysctl and one bug fix to a
sysctl strategy routine I intend to delete.

The core or my changes are in.  This is just purging dead code.

It looks like those two changes may actually be single patches so I
could cherry pick them and in theory have no conflicts.

At the very least I'm going to wait for one more build of net-next with
everything but my network stack changes before I add those.

So far the conflicts are pretty minimal so I guess it doesn't matter
much.

Eric

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Joe Perches]

On Sun, 2009-11-15 at 11:33 +0100, Ingo Molnar wrote:
> * Eric W. Biederman <ebiederm@xmission.com> wrote:
> > Ingo Molnar <mingo@elte.hu> writes:
> > > * Joe Perches <joe@perches.com> wrote:
> > >> On Sun, 2009-11-15 at 09:11 +0100, Ingo Molnar wrote:
> > >> > * Am??rico Wang <xiyou.wangcong@gmail.com> wrote:
> > >> > > On Sat, Nov 14, 2009 at 05:52:05PM -0800, Joe Perches wrote:
> > >> > > >Seems to be a typo.
> > >> > > Acked-by: WANG Cong <xiyou.wangcong@gmail.com>
> > >> > (Cc:-ed Eric who is running the sysctl tree these days)
> > >> > Almost everywhere in the kernel we use the shorter version, so all of 
> > >> > sysctl.c should eventually change to that variant.
> > >> It's closer to 50/50, but it's 1 vs 133 in that file.
> > >> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*&\s*\w+' * | wc -l
> > >> 339
> > >> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*[^&]\s*\w+' * | wc -l
> > >> 432
> > > I did not mean this specific initialization method of proc_handler, i 
> > > meant pointers to functions in general.
> > There was an argument put forward by Alexy (I think) a while ago.  
> > That argued for the form without the address of operator.
> > The reason being that without it you can do:
> > #define proc_dointvec NULL
> > in a header when sysctl support it compiled out.  Using address of
> > you wind up with stub functions in sysctl.c to handle the case when
> > sysctl is compiled out.
> > It isn't a strong case but since not using & is also shorter and as 
> > Ingo pointed out more common I think no & wins.
> I can think of another reason as well: the & operator can be dangerous 
> if code is changed from functions to function pointers.
> 
> The short form:
> 
>   val = do_my_func;
> 
> will work just fine if 'my_func' is changed to a function pointer, as it 
> will evaluate to the value of the function pointer - i.e. the address of 
> the function.
> 
> The longer form:
> 
>   val = &do_my_func;
> 
> might break in a subtle way, because it will now become the address of 
> the function pointer - not the function address.
> 
> Combined the shortness, the NULL init, the function pointer invariance, 
> plus existing in-kernel practice all suggest that the short form should 
> be used.
> 
> ( i didnt want to turn this small issue into a long argument - it's just
>   that the code was going in the wrong direction. )

That sounds like something coccinelle would do well,
so I've cc'd Julia Lawall.

http://lkml.org/lkml/2009/11/15/55

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Julia Lawall]

On Sun, 15 Nov 2009, Joe Perches wrote:

> On Sun, 2009-11-15 at 11:33 +0100, Ingo Molnar wrote:
> > * Eric W. Biederman <ebiederm@xmission.com> wrote:
> > > Ingo Molnar <mingo@elte.hu> writes:
> > > > * Joe Perches <joe@perches.com> wrote:
> > > >> On Sun, 2009-11-15 at 09:11 +0100, Ingo Molnar wrote:
> > > >> > * Am??rico Wang <xiyou.wangcong@gmail.com> wrote:
> > > >> > > On Sat, Nov 14, 2009 at 05:52:05PM -0800, Joe Perches wrote:
> > > >> > > >Seems to be a typo.
> > > >> > > Acked-by: WANG Cong <xiyou.wangcong@gmail.com>
> > > >> > (Cc:-ed Eric who is running the sysctl tree these days)
> > > >> > Almost everywhere in the kernel we use the shorter version, so all of 
> > > >> > sysctl.c should eventually change to that variant.
> > > >> It's closer to 50/50, but it's 1 vs 133 in that file.
> > > >> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*&\s*\w+' * | wc -l
> > > >> 339
> > > >> $ grep -Pr --include=*.[ch] '\.proc_handler\s*=\s*[^&]\s*\w+' * | wc -l
> > > >> 432
> > > > I did not mean this specific initialization method of proc_handler, i 
> > > > meant pointers to functions in general.
> > > There was an argument put forward by Alexy (I think) a while ago.  
> > > That argued for the form without the address of operator.
> > > The reason being that without it you can do:
> > > #define proc_dointvec NULL
> > > in a header when sysctl support it compiled out.  Using address of
> > > you wind up with stub functions in sysctl.c to handle the case when
> > > sysctl is compiled out.
> > > It isn't a strong case but since not using & is also shorter and as 
> > > Ingo pointed out more common I think no & wins.
> > I can think of another reason as well: the & operator can be dangerous 
> > if code is changed from functions to function pointers.
> > 
> > The short form:
> > 
> >   val = do_my_func;
> > 
> > will work just fine if 'my_func' is changed to a function pointer, as it 
> > will evaluate to the value of the function pointer - i.e. the address of 
> > the function.
> > 
> > The longer form:
> > 
> >   val = &do_my_func;
> > 
> > might break in a subtle way, because it will now become the address of 
> > the function pointer - not the function address.
> > 
> > Combined the shortness, the NULL init, the function pointer invariance, 
> > plus existing in-kernel practice all suggest that the short form should 
> > be used.
> > 
> > ( i didnt want to turn this small issue into a long argument - it's just
> >   that the code was going in the wrong direction. )
> 
> That sounds like something coccinelle would do well,
> so I've cc'd Julia Lawall.
> 
> http://lkml.org/lkml/2009/11/15/55

Searching for things that are declared as functions (either a definition 
or a prototype), and then referenced as &f gives over 2000 results in 
almost 600 files.  Here are a couple of typical examples:

arch/arm/mach-omap1/clock.c:

static const struct clkops clkops_dspck = {
       .enable         = &omap1_clk_enable_dsp_domain,
       .disable        = &omap1_clk_disable_dsp_domain,
 };


arch/arm/mach-omap1/serial.c:

       ret = request_irq(gpio_to_irq(gpio_nr), &omap_serial_wake_interrupt,
                          IRQF_TRIGGER_RISING, "serial wakeup", NULL);

Should both cases lose the initial &?

julia

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Joe Perches]

On Sun, 2009-11-15 at 19:20 +0100, Julia Lawall wrote:
> Searching for things that are declared as functions (either a definition 
> or a prototype), and then referenced as &f gives over 2000 results in 
> almost 600 files.

Just curious, do you know how many are referenced
without the &?

> Here are a couple of typical examples:
> 
> arch/arm/mach-omap1/clock.c:
> 
> static const struct clkops clkops_dspck = {
>        .enable         = &omap1_clk_enable_dsp_domain,
>        .disable        = &omap1_clk_disable_dsp_domain,
>  };
> 
> arch/arm/mach-omap1/serial.c:
> 
>        ret = request_irq(gpio_to_irq(gpio_nr), &omap_serial_wake_interrupt,
>                           IRQF_TRIGGER_RISING, "serial wakeup", NULL);
> 
> Should both cases lose the initial &?

If what is desired is kernel wide consistent use, yes.
What I would like is file/subsystem consistent use.

Looking at sysctl.c and seeing that different use
stand out was odd.

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Julia Lawall]

On Sun, 15 Nov 2009, Joe Perches wrote:

> On Sun, 2009-11-15 at 19:20 +0100, Julia Lawall wrote:
> > Searching for things that are declared as functions (either a definition 
> > or a prototype), and then referenced as &f gives over 2000 results in 
> > almost 600 files.
> 
> Just curious, do you know how many are referenced
> without the &?

I got over 95000 (not checked in detail, though).

> > Here are a couple of typical examples:
> > 
> > arch/arm/mach-omap1/clock.c:
> > 
> > static const struct clkops clkops_dspck = {
> >        .enable         = &omap1_clk_enable_dsp_domain,
> >        .disable        = &omap1_clk_disable_dsp_domain,
> >  };
> > 
> > arch/arm/mach-omap1/serial.c:
> > 
> >        ret = request_irq(gpio_to_irq(gpio_nr), &omap_serial_wake_interrupt,
> >                           IRQF_TRIGGER_RISING, "serial wakeup", NULL);
> > 
> > Should both cases lose the initial &?
> 
> If what is desired is kernel wide consistent use, yes.
> What I would like is file/subsystem consistent use.
> 
> Looking at sysctl.c and seeing that different use
> stand out was odd.

It would be possible to count the number of occurrences in a given file, 
and then change the ones that have the less popular format, or a format 
that occurs less than some percentage of time.

julia

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Arnd Bergmann]

On Sunday 15 November 2009 20:40:25 Julia Lawall wrote:
> It would be possible to count the number of occurrences in a given file, 
> and then change the ones that have the less popular format, or a format 
> that occurs less than some percentage of time.

How many of the 600 files use both formats?

	Arnd <><

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Julia Lawall]

On Sun, 15 Nov 2009, Arnd Bergmann wrote:

> On Sunday 15 November 2009 20:40:25 Julia Lawall wrote:
> > It would be possible to count the number of occurrences in a given file, 
> > and then change the ones that have the less popular format, or a format 
> > that occurs less than some percentage of time.
> 
> How many of the 600 files use both formats?

This time I considered only cases where the function is defined in the 
same file, not where the function is presented only as a prototype.  I 
get:

502 files that use both options
83 files that only use & for function pointers
9374 files that never use & for function pointers

(Only files that use at least one of the options are considered.)

julia

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Joe Perches]

On Sun, 2009-11-15 at 21:40 +0100, Julia Lawall wrote:
> On Sun, 15 Nov 2009, Joe Perches wrote:
> > On Sun, 2009-11-15 at 19:20 +0100, Julia Lawall wrote:
> > > Searching for things that are declared as functions (either a definition 
> > > or a prototype), and then referenced as &f gives over 2000 results in 
> > > almost 600 files.
> > Just curious, do you know how many are referenced
> > without the &?
> I got over 95000 (not checked in detail, though).

Thanks.

I think that ~25+:1 ratio makes a good case for an
eventual conversion of the remaining uses.

> > If what is desired is kernel wide consistent use, yes.
> > What I would like is file/subsystem consistent use.
> > 
> > Looking at sysctl.c and seeing that different use
> > stand out was odd.
> 
> It would be possible to count the number of occurrences in a given file, 
> and then change the ones that have the less popular format, or a format 
> that occurs less than some percentage of time.

Maybe it'd be useful to add a coccinelle/spatch
directory in scripts and add these scripts so
that files and subsystems can updated over time.

I can not find a directory of coccinelle input
scripts for linux at:
http://coccinelle.lip6.fr/download.php
Is there a list somewhere?

Perhaps add a checkpatch test as well though
that might be an interesting test to write in
perl.

Re: [PATCH] sysctl.c: Change a .proc_handler = proc_dointvec to &proc_dointvec,

[Julia Lawall]

On Sun, 15 Nov 2009, Joe Perches wrote:

> On Sun, 2009-11-15 at 21:40 +0100, Julia Lawall wrote:
> > On Sun, 15 Nov 2009, Joe Perches wrote:
> > > On Sun, 2009-11-15 at 19:20 +0100, Julia Lawall wrote:
> > > > Searching for things that are declared as functions (either a definition 
> > > > or a prototype), and then referenced as &f gives over 2000 results in 
> > > > almost 600 files.
> > > Just curious, do you know how many are referenced
> > > without the &?
> > I got over 95000 (not checked in detail, though).
> 
> Thanks.
> 
> I think that ~25+:1 ratio makes a good case for an
> eventual conversion of the remaining uses.

OK, I will try to look into it sometime soon.

> > > If what is desired is kernel wide consistent use, yes.
> > > What I would like is file/subsystem consistent use.
> > > 
> > > Looking at sysctl.c and seeing that different use
> > > stand out was odd.
> > 
> > It would be possible to count the number of occurrences in a given file, 
> > and then change the ones that have the less popular format, or a format 
> > that occurs less than some percentage of time.
> 
> Maybe it'd be useful to add a coccinelle/spatch
> directory in scripts and add these scripts so
> that files and subsystems can updated over time.
> 
> I can not find a directory of coccinelle input
> scripts for linux at:
> http://coccinelle.lip6.fr/download.php
> Is there a list somewhere?

At the bottom of the download page there is a tool called coccicheck that 
runs Coccinelle on a set of semantic patches.  It comes with a set of 
examples.

http://coccinelle.lip6.fr/distrib/coccicheck-0.1.tgz

julia

> Perhaps add a checkpatch test as well though
> that might be an interesting test to write in
> perl.
> 
> 
>