GCC nested functions?

GCC nested functions?

[Stephen Hemminger]

I used GCC nested functions in the (not released) bridge sysfs interface for 2.6.6.
It seemed like a nice way to express the sysfs related interface without doing
lots of code copying (or worse lots of macros).

The code in question looks like:
static ssize_t store_bridge_parm(struct class_device *cd,
                                 const char *buf, size_t len,
                                 void (*store)(struct net_bridge *, unsigned long))
{
        struct net_bridge *br = to_bridge(cd);
        char *endp;
        unsigned long val;
                                                                                
        if (!capable(CAP_NET_ADMIN))
                return -EPERM;
                                                                                
        val = simple_strtoul(buf, &endp, 0);
        if (endp == buf)
                return -EINVAL;
                                                                                
        spin_lock_bh(&br->lock);
        store(br, val);
        spin_unlock_bh(&br->lock);
        return len;
}
...

static ssize_t store_forward_delay(struct class_device *cd, const char *buf,
                                   size_t len)
{
        void store(struct net_bridge *br, unsigned long val)
        {
                unsigned long delay = clock_t_to_jiffies(val);
                br->forward_delay = delay;
                if (br_is_root_bridge(br))
                        br->bridge_forward_delay = delay;
        }
                                                                                
        return store_bridge_parm(cd, buf, len, store);
}


This works fine for GCC 2.95 and 3.X for i386 and x86_64 architectures, but the ia64
(cross compiler) pukes with:

 In function `store_forward_delay':
: undefined reference to `__ia64_trampoline'

Redoing it as separate functions is easy enough, but the questions are:
	- Are gcc nested functions allowed in the kernel?  If not where should
	  this restriction be put in Documentation? CodingStyles?
	- Or is gcc on ia64 just too stupid? or do some more support routines
	  need to exist in arch/ia64?
	- Do other architectures (sparc, ppc) have similar problems?

Thanks.

Re: GCC nested functions?

[Christoph Hellwig]

On Wed, May 12, 2004 at 10:59:24AM -0700, Stephen Hemminger wrote:
> Redoing it as separate functions is easy enough, but the questions are:
> 	- Are gcc nested functions allowed in the kernel?  If not where should
> 	  this restriction be put in Documentation? CodingStyles?

nested function are a horrible gcc misfeature.  So far people had enough
taste to not introduce them without explicitly forbidding it ;-)

Maybe we should add a section to Documentation/CodingStyles that says
which gcc extensions are okay for kernel use.

> 	- Or is gcc on ia64 just too stupid? or do some more support routines
> 	  need to exist in arch/ia64?
> 	- Do other architectures (sparc, ppc) have similar problems?

There's a few architectures needing libgcc help for trampolines, but I
don't remember which ones exactly.

Re: GCC nested functions?

[Mitchell Blank Jr]

Christoph Hellwig wrote:
> nested function are a horrible gcc misfeature.

True.

> So far people had enough
> taste to not introduce them without explicitly forbidding it ;-)

Almost true.  Look at drivers/atm/horizon.c:make_rate(), which includes
a nested function called "set_cr()"  There may be a couple other
examples in the drivers/atm directory, not sure.

The Intel CC guys made special mention of the ATM drivers when they
first started using their compiler for the kernel (icc supports most
gcc extensions, but apparently not nested functions) and they made special
mention of not being able to compile some ATM drivers because of this.

I'm not aware of nested functions anywhere else in the kernel though.

-Mitch

Re: GCC nested functions?

[William Lee Irwin III]

On Wed, May 12, 2004 at 10:59:24AM -0700, Stephen Hemminger wrote:
> I used GCC nested functions in the (not released) bridge sysfs
> interface for 2.6.6. It seemed like a nice way to express the sysfs
> related interface without doing lots of code copying (or worse lots
> of macros).

Upward funargs are rather easy to trip over (though you haven't tripped
over them) and the runtime shits itself badly on all architectures when
they happen. It's too dangerous to be allowed to be used.

The code you posted doesn't really have a reason to use nested functions
(i.e. downward funargs aren't being used).


-- wli

Re: GCC nested functions?

[David Mosberger]

>>>>> On Wed, 12 May 2004 10:59:24 -0700, Stephen Hemminger <shemminger@osdl.org> said:

  Stephen> I used GCC nested functions in the (not released) bridge
  Stephen> sysfs interface for 2.6.6.  It seemed like a nice way to
  Stephen> express the sysfs related interface without doing lots of
  Stephen> code copying (or worse lots of macros).

Oh, man!  Nested C functions are evil.  Just don't do it.

  Stephen> This works fine for GCC 2.95 and 3.X for i386 and x86_64
  Stephen> architectures, but the ia64 (cross compiler) pukes with:

  Stephen> In function `store_forward_delay':
  Stephen> : undefined reference to `__ia64_trampoline'

  Stephen> Redoing it as separate functions is easy enough, but the
  Stephen> questions are:

  Stephen> - Are gcc nested functions allowed in the kernel?  If not
  Stephen> where should this restriction be put in Documentation?
  Stephen> CodingStyles?

Nested C functions shouldn't be allowed _anywhere_.  It's the worst
extension that has made it into GNU C.

  Stephen> - Or is gcc on ia64 just too stupid? or do some more
  Stephen> support routines need to exist in arch/ia64?

It has nothing to do with stupidity.  The kernel doesn't support all
the routines provided by libgcc.a.  __ia64_trampoline() is one of
them.

  Stephen> - Do other architectures (sparc, ppc) have similar problems?

It's not a problem.  It's a feature.  It's likely that other
architectures which require a helper-routine from libgcc would behave
the same.

	--david

Re: GCC nested functions?

[viro]

On Wed, May 12, 2004 at 10:59:24AM -0700, Stephen Hemminger wrote:
> I used GCC nested functions in the (not released) bridge sysfs interface for 2.6.6.
> It seemed like a nice way to express the sysfs related interface without doing
> lots of code copying (or worse lots of macros).

If nested functions are answer, you've got the wrong question...

Re: GCC nested functions?

[Gabriel Paubert]

On Wed, May 12, 2004 at 10:59:24AM -0700, Stephen Hemminger wrote:
> I used GCC nested functions in the (not released) bridge sysfs interface for 2.6.6.
> It seemed like a nice way to express the sysfs related interface without doing
> lots of code copying (or worse lots of macros).
> 
> The code in question looks like:
> static ssize_t store_bridge_parm(struct class_device *cd,
>                                  const char *buf, size_t len,
>                                  void (*store)(struct net_bridge *, unsigned long))
> {
>         struct net_bridge *br = to_bridge(cd);
>         char *endp;
>         unsigned long val;
>                                                                                 
>         if (!capable(CAP_NET_ADMIN))
>                 return -EPERM;
>                                                                                 
>         val = simple_strtoul(buf, &endp, 0);
>         if (endp == buf)
>                 return -EINVAL;
>                                                                                 
>         spin_lock_bh(&br->lock);
>         store(br, val);
>         spin_unlock_bh(&br->lock);
>         return len;
> }
> ...
> 
> static ssize_t store_forward_delay(struct class_device *cd, const char *buf,
>                                    size_t len)
> {
>         void store(struct net_bridge *br, unsigned long val)
>         {
>                 unsigned long delay = clock_t_to_jiffies(val);
>                 br->forward_delay = delay;
>                 if (br_is_root_bridge(br))
>                         br->bridge_forward_delay = delay;
>         }
>                                                                                 
>         return store_bridge_parm(cd, buf, len, store);
> }
> 
> 
> This works fine for GCC 2.95 and 3.X for i386 and x86_64 architectures, but the ia64
> (cross compiler) pukes with:
> 
>  In function `store_forward_delay':
> : undefined reference to `__ia64_trampoline'
> 
> Redoing it as separate functions is easy enough, but the questions are:
> 	- Are gcc nested functions allowed in the kernel?  If not where should
> 	  this restriction be put in Documentation? CodingStyles?

There is some kind of implicit prohibition in Documentation/CodingStyle
(Chapter3: Placing braces):

"Heretic people all over the world have claimed that this inconsistency
is ...  well ...  inconsistent, but all right-thinking people know that
(a) K&R are _right_ and (b) K&R are right.  Besides, functions are
special anyway (you can't nest them in C)."
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^

Maybe the way it is stated is too close so subliminal ;-) But I never found 
a real need for them in C.

	Gabriel