From: "Jérôme Pouiller" <j.pouiller@sysmic.fr>
To: linuxppc-dev@lists.ozlabs.org, Grant Likely <grant.likely@secretlab.ca>
Cc: netdev <netdev@vger.kernel.org>,
Andy Fleming <afleming@freescale.com>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: Nested function in drivers/of/of_mdio.c
Date: Thu, 8 Oct 2009 10:45:12 +0200 [thread overview]
Message-ID: <200910081045.12590.j.pouiller@sysmic.fr> (raw)
In-Reply-To: <f608b67d0910070923h54c30c2doae08550b0791ed1b@mail.gmail.com>
I did some grep on codebase. I have not found any other instances of=20
nested functions, but my regexps are not enough to be 100% sure.
On Wednesday 07 October 2009 18:23:04 vb@vsbe.com wrote:
> Guys, are there other instances of nested C functions in the codebase
> or was this the first attempt?
>=20
> On Wed, Oct 7, 2009 at 9:11 AM, Grant Likely=20
<grant.likely@secretlab.ca> wrote:
> > On Wed, Oct 7, 2009 at 9:15 AM, J=E9r=F4me Pouiller <jezz@sysmic.org>=20
wrote:
> >> Dear,
> >>
> >> I have a problem with commit
> >> 8bc487d150b939e69830c39322df4ee486efe381 in file
> >> drivers/of/of_mdio.c in function of_phy_find_device.
> >>
> >> As you see, this function define match() as a nested function. My
> >> compiler (powerpc-e500-linux-gnu-gcc-3.4.1) raise an error during
> >> link due to this nested definition:
> >> drivers/built-in.o(.text+0x5e2a4): In function
> >> `of_phy_find_device':
> >> /home/jezz/linux-next/drivers/of/of_mdio.c:107: undefined
> >> reference to `__trampoline_setup'
> >>
> >> I am sure I could solve problem by rebuilding my toolchain.
> >> Nevertheless, I think nested function definition is not perfectly
> >> supported by all compilers. Also, I suggest to place function
> >> match() outside of scope of of_phy_find_device as in following
> >> patch.
> >
> > I'm okay with that, but if you're moving code out of the file
> > scope, then please rename the function to of_phy_match() to avoid
> > global namespace conflicts.
> >
> > g.
> >
> >> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> >> index bacaa53..c7b2e26 100644
> >> --- a/drivers/of/of_mdio.c
> >> +++ b/drivers/of/of_mdio.c
> >> @@ -97,6 +97,10 @@ int of_mdiobus_register(struct mii_bus *mdio,
> >> struct device_node *np) }
> >> EXPORT_SYMBOL(of_mdiobus_register);
> >>
> >> +static int match(struct device *dev, void *phy_np)
> >> +{
> >> + return dev_archdata_get_node(&dev->archdata) =3D=3D phy_np;
> >> +}
> >> /**
> >> * of_phy_find_device - Give a PHY node, find the phy_device
> >> * @phy_np: Pointer to the phy's device tree node
> >> @@ -106,11 +110,6 @@ EXPORT_SYMBOL(of_mdiobus_register);
> >> struct phy_device *of_phy_find_device(struct device_node *phy_np)
> >> {
> >> struct device *d;
> >> - int match(struct device *dev, void *phy_np)
> >> - {
> >> - return dev_archdata_get_node(&dev->archdata) =3D=3D
> >> phy_np; - }
> >> -
> >> if (!phy_np)
> >> return NULL;
> >>
> >>
> >> What do you think about it?
> >>
> >> Best regards,
> >>
> >> --
> >> J=E9r=F4me Pouiller (jezz AT sysmic DOT org)
> >
> > --
> > Grant Likely, B.Sc., P.Eng.
> > Secret Lab Technologies Ltd.
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/linuxppc-dev
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>=20
=2D-=20
J=E9r=F4me Pouiller (jerome AT sysmic DOT org)
Expert Linux Embarqu=E9
WARNING: multiple messages have this Message-ID (diff)
From: "Jérôme Pouiller" <j.pouiller@sysmic.fr>
To: linuxppc-dev@lists.ozlabs.org, Grant Likely <grant.likely@secretlab.ca>
Cc: netdev <netdev@vger.kernel.org>,
Andy Fleming <afleming@freescale.com>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: Nested function in drivers/of/of_mdio.c
Date: Thu, 8 Oct 2009 10:45:12 +0200 [thread overview]
Message-ID: <200910081045.12590.j.pouiller@sysmic.fr> (raw)
In-Reply-To: <f608b67d0910070923h54c30c2doae08550b0791ed1b@mail.gmail.com>
I did some grep on codebase. I have not found any other instances of
nested functions, but my regexps are not enough to be 100% sure.
On Wednesday 07 October 2009 18:23:04 vb@vsbe.com wrote:
> Guys, are there other instances of nested C functions in the codebase
> or was this the first attempt?
>
> On Wed, Oct 7, 2009 at 9:11 AM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> > On Wed, Oct 7, 2009 at 9:15 AM, Jérôme Pouiller <jezz@sysmic.org>
wrote:
> >> Dear,
> >>
> >> I have a problem with commit
> >> 8bc487d150b939e69830c39322df4ee486efe381 in file
> >> drivers/of/of_mdio.c in function of_phy_find_device.
> >>
> >> As you see, this function define match() as a nested function. My
> >> compiler (powerpc-e500-linux-gnu-gcc-3.4.1) raise an error during
> >> link due to this nested definition:
> >> drivers/built-in.o(.text+0x5e2a4): In function
> >> `of_phy_find_device':
> >> /home/jezz/linux-next/drivers/of/of_mdio.c:107: undefined
> >> reference to `__trampoline_setup'
> >>
> >> I am sure I could solve problem by rebuilding my toolchain.
> >> Nevertheless, I think nested function definition is not perfectly
> >> supported by all compilers. Also, I suggest to place function
> >> match() outside of scope of of_phy_find_device as in following
> >> patch.
> >
> > I'm okay with that, but if you're moving code out of the file
> > scope, then please rename the function to of_phy_match() to avoid
> > global namespace conflicts.
> >
> > g.
> >
> >> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> >> index bacaa53..c7b2e26 100644
> >> --- a/drivers/of/of_mdio.c
> >> +++ b/drivers/of/of_mdio.c
> >> @@ -97,6 +97,10 @@ int of_mdiobus_register(struct mii_bus *mdio,
> >> struct device_node *np) }
> >> EXPORT_SYMBOL(of_mdiobus_register);
> >>
> >> +static int match(struct device *dev, void *phy_np)
> >> +{
> >> + return dev_archdata_get_node(&dev->archdata) == phy_np;
> >> +}
> >> /**
> >> * of_phy_find_device - Give a PHY node, find the phy_device
> >> * @phy_np: Pointer to the phy's device tree node
> >> @@ -106,11 +110,6 @@ EXPORT_SYMBOL(of_mdiobus_register);
> >> struct phy_device *of_phy_find_device(struct device_node *phy_np)
> >> {
> >> struct device *d;
> >> - int match(struct device *dev, void *phy_np)
> >> - {
> >> - return dev_archdata_get_node(&dev->archdata) ==
> >> phy_np; - }
> >> -
> >> if (!phy_np)
> >> return NULL;
> >>
> >>
> >> What do you think about it?
> >>
> >> Best regards,
> >>
> >> --
> >> Jérôme Pouiller (jezz AT sysmic DOT org)
> >
> > --
> > Grant Likely, B.Sc., P.Eng.
> > Secret Lab Technologies Ltd.
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/linuxppc-dev
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
--
Jérôme Pouiller (jerome AT sysmic DOT org)
Expert Linux Embarqué
next prev parent reply other threads:[~2009-10-08 8:51 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-10-07 15:15 Nested function in drivers/of/of_mdio.c Jérôme Pouiller
2009-10-07 15:15 ` Jérôme Pouiller
2009-10-07 16:11 ` Grant Likely
2009-10-07 16:11 ` Grant Likely
2009-10-07 16:23 ` vb
2009-10-07 16:23 ` vb
2009-10-08 8:45 ` Jérôme Pouiller [this message]
2009-10-08 8:45 ` Jérôme Pouiller
2009-10-08 11:14 ` Gabriel Paubert
2009-10-08 8:34 ` [PATCH] Remove nested function Jérôme Pouiller
2009-10-08 8:34 ` Jérôme Pouiller
2009-10-08 9:10 ` David Miller
2009-10-08 9:10 ` David Miller
[not found] <200910071629.15804.jezz@sysmic.org>
2009-10-07 14:55 ` Nested function in drivers/of/of_mdio.c Grant Likely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200910081045.12590.j.pouiller@sysmic.fr \
--to=j.pouiller@sysmic.fr \
--cc=afleming@freescale.com \
--cc=davem@davemloft.net \
--cc=grant.likely@secretlab.ca \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.