2002-07-06 08:28:06

by Keith Owens

[permalink] [raw]
Subject: [patch] 2.5.25 net/core/Makefile

The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
builds ext8022.o and gets unresolved references because there is no
network code. Detected by kbuild 2.5, not detected by the existing
build system.

Index: 25.1/net/core/Makefile
--- 25.1/net/core/Makefile Wed, 19 Jun 2002 14:11:35 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
+++ 25.1(w)/net/core/Makefile Sat, 06 Jul 2002 18:27:16 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
@@ -16,7 +16,7 @@ obj-$(CONFIG_FILTER) += filter.o

obj-$(CONFIG_NET) += dev.o dev_mcast.o dst.o neighbour.o rtnetlink.o utils.o

-ifneq ($(CONFIG_LLC),n)
+ifneq ($(subst n,,$(CONFIG_LLC)),)
obj-y += ext8022.o
endif



2002-07-09 00:09:30

by Dave Jones

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

On Sat, Jul 06, 2002 at 06:30:29PM +1000, Keith Owens wrote:
> The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
> builds ext8022.o and gets unresolved references because there is no
> network code. Detected by kbuild 2.5, not detected by the existing
> build system.

And this breaks the valid combination of CONFIG_NET=y, CONFIG_LLC undef'd

net/network.o: In function Register_8022_client':
net/network.o(.text+0xe8b7): undefined reference to Llc_register_sap'
net/network.o: In function Unregister_8022_client':
net/network.o(.text+0xe8fe): undefined reference to Llc_unregister_sap'


--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2002-07-09 00:28:02

by Keith Owens

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

On Tue, 9 Jul 2002 01:11:35 +0100,
Dave Jones <[email protected]> wrote:
>On Sat, Jul 06, 2002 at 06:30:29PM +1000, Keith Owens wrote:
> > The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
> > builds ext8022.o and gets unresolved references because there is no
> > network code. Detected by kbuild 2.5, not detected by the existing
> > build system.
>
>And this breaks the valid combination of CONFIG_NET=y, CONFIG_LLC undef'd
>
>net/network.o: In function Register_8022_client':
>net/network.o(.text+0xe8b7): undefined reference to Llc_register_sap'
>net/network.o: In function Unregister_8022_client':
>net/network.o(.text+0xe8fe): undefined reference to Llc_unregister_sap'

??? That is the bug that I reported. My patch fixes that bug.

Index: 25.1/net/core/Makefile
--- 25.1/net/core/Makefile Wed, 19 Jun 2002 14:11:35 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
+++ 25.1(w)/net/core/Makefile Sat, 06 Jul 2002 18:27:16 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
@@ -16,7 +16,7 @@ obj-$(CONFIG_FILTER) += filter.o

obj-$(CONFIG_NET) += dev.o dev_mcast.o dst.o neighbour.o rtnetlink.o utils.o

-ifneq ($(CONFIG_LLC),n)
+ifneq ($(subst n,,$(CONFIG_LLC)),)
obj-y += ext8022.o
endif


2002-07-09 00:33:49

by Dave Jones

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

On Tue, Jul 09, 2002 at 10:30:31AM +1000, Keith Owens wrote:
> > > The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
> >And this breaks the valid combination of CONFIG_NET=y, CONFIG_LLC undef'd

> ??? That is the bug that I reported. My patch fixes that bug.

Same bug maybe, but triggered in different ways.
Note the CONFIG_NET change between your report and mine.

With mainline kbuild this isn't an issue, but your fix breaks it.

*shrug*

Dave

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2002-07-09 02:11:06

by Keith Owens

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

On Tue, 9 Jul 2002 02:36:28 +0200,
Dave Jones <[email protected]> wrote:
>On Tue, Jul 09, 2002 at 10:30:31AM +1000, Keith Owens wrote:
> > > > The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
> > >And this breaks the valid combination of CONFIG_NET=y, CONFIG_LLC undef'd
>
> > ??? That is the bug that I reported. My patch fixes that bug.
>
>Same bug maybe, but triggered in different ways.
>Note the CONFIG_NET change between your report and mine.

Sorry, missed that change the first time.

The problem is net/802/Makefile which includes p8022 for any of
CONFIG_LLC, CONFIG_TR, CONFIG_IPX or CONFIG_ATALK. p8022 calls
llc_register_sap which is in ext8022.o, that file is built by
net/core/Makefile but only for CONFIG_LLC. It worked before because of
the wrong test in net/core/Makefile which always built ext8022.o.

Davem - we could unconditionally build ext8022.o when CONFIG_NET=y or
we could do this

Index: 25.1/net/core/Makefile
--- 25.1/net/core/Makefile Wed, 19 Jun 2002 14:11:35 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
+++ 25.1(w)/net/core/Makefile Tue, 09 Jul 2002 12:10:53 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
@@ -16,7 +16,8 @@ obj-$(CONFIG_FILTER) += filter.o

obj-$(CONFIG_NET) += dev.o dev_mcast.o dst.o neighbour.o rtnetlink.o utils.o

-ifneq ($(CONFIG_LLC),n)
+# See p8022 in net/802/Makefile for config options to check
+ifneq ($(subst n,,$(CONFIG_LLC)$(CONFIG_TR)$(CONFIG_IPX)$(CONFIG_ATALK)),)
obj-y += ext8022.o
endif

Index: 25.1/net/802/Makefile
--- 25.1/net/802/Makefile Fri, 21 Jun 2002 10:09:01 +1000 kaos (linux-2.5/r/c/0_Makefile 1.3 444)
+++ 25.1(w)/net/802/Makefile Tue, 09 Jul 2002 12:10:38 +1000 kaos (linux-2.5/r/c/0_Makefile 1.3 444)
@@ -6,6 +6,7 @@ export-objs := llc_macinit.o p8022.o ps

obj-y := p8023.o

+# Check the p8022 selections against net/core/Makefile.
obj-$(CONFIG_SYSCTL) += sysctl_net_802.o
obj-$(CONFIG_LLC) += p8022.o psnap.o llc_sendpdu.o llc_utility.o \
cl2llc.o llc_macinit.o



2002-07-09 02:22:29

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

Em Tue, Jul 09, 2002 at 12:13:30PM +1000, Keith Owens escreveu:
> On Tue, 9 Jul 2002 02:36:28 +0200,
> Dave Jones <[email protected]> wrote:
> >Same bug maybe, but triggered in different ways.
> >Note the CONFIG_NET change between your report and mine.
>
> Sorry, missed that change the first time.
>
> The problem is net/802/Makefile which includes p8022 for any of
> CONFIG_LLC, CONFIG_TR, CONFIG_IPX or CONFIG_ATALK. p8022 calls

This will be moot when I remove p8022.c from the kernel (in fact all of
net/802), which I plan to do before 2.5 freeze 8)

- Arnaldo

2002-07-11 03:50:50

by Thunder from the hill

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

Hi,

On Jul 9, 2002 02:13:30, Keith Owens wrote:
> +# See p8022 in net/802/Makefile for config options to check
> +ifneq ($(subst n,,$(CONFIG_LLC)$(CONFIG_TR)$(CONFIG_IPX)$(CONFIG_ATALK)),)
> obj-y += ext8022.o
> endif

Make's response:

make[4]: Entering directory `/home/thunder/tmp/thunder-2.5-kb24/net/core'
Makefile:20: *** missing separator. Stop.
make[4]: Leaving directory `/home/thunder/tmp/thunder-2.5-kb24/net/core'
make[3]: *** [core] Error 2

Ideas?

Regards,
Thunder
--
(Use http://www.ebb.org/ungeek if you can't decode)
------BEGIN GEEK CODE BLOCK------
Version: 3.12
GCS/E/G/S/AT d- s++:-- a? C++$ ULAVHI++++$ P++$ L++++(+++++)$ E W-$
N--- o? K? w-- O- M V$ PS+ PE- Y- PGP+ t+ 5+ X+ R- !tv b++ DI? !D G
e++++ h* r--- y-
------END GEEK CODE BLOCK------

2002-07-22 08:04:09

by Russell King

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

On Mon, Jul 22, 2002 at 11:08:41AM +1000, Keith Owens wrote:
> It is required if you ever want autoconfigure to work, that
> distinguishes between "" (undefined) and "n" (explicitly turned off).
> Forward planning.

Wouldn't it be better to fix the existing config tools to output "=n"
instead of "# CONFIG_foo is not set" ? IIRC they do the translation
back and forth internally anyway, so it should be just a matter of
removing some code from the tools.

After all, the earlier we update the config tools, the earlier we can
do something with the makefiles (after a reasonable period for things
like mconfig to catch up...)

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2002-07-21 23:53:59

by Kai Germaschewski

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

On Sat, 6 Jul 2002, Keith Owens wrote:

> The valid combination of CONFIG_NET=n, CONFIG_LLC undefined incorrectly
> builds ext8022.o and gets unresolved references because there is no
> network code. Detected by kbuild 2.5, not detected by the existing
> build system.
>
> Index: 25.1/net/core/Makefile
> --- 25.1/net/core/Makefile Wed, 19 Jun 2002 14:11:35 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
> +++ 25.1(w)/net/core/Makefile Sat, 06 Jul 2002 18:27:16 +1000 kaos (linux-2.5/p/c/50_Makefile 1.4 444)
> @@ -16,7 +16,7 @@ obj-$(CONFIG_FILTER) += filter.o
>
> obj-$(CONFIG_NET) += dev.o dev_mcast.o dst.o neighbour.o rtnetlink.o utils.o
>
> -ifneq ($(CONFIG_LLC),n)
> +ifneq ($(subst n,,$(CONFIG_LLC)),)
> obj-y += ext8022.o
> endif

Makes sense to me. However, the CONFIG_ variables used in the Makefiles
are never "n", they are "y", "m" or undefined.

In Config.in scripts you have to cater for "n" or "", and I've seen
various people on l-k carry this behavior into the Makefiles, but there
it's unnecessary for all I can tell.

So

ifdef CONFIG_LLC
obj-y += ext8022.o
endif

should do the job just fine.

--Kai


2002-07-22 08:35:27

by Roman Zippel

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

Hi,

On Mon, 22 Jul 2002, Russell King wrote:

> Wouldn't it be better to fix the existing config tools to output "=n"
> instead of "# CONFIG_foo is not set" ? IIRC they do the translation
> back and forth internally anyway, so it should be just a matter of
> removing some code from the tools.

This would mean, tristate symbols had four states instead of three. The
current shell based config systems simply don't see all symbols.
Depending on the configuration a symbol could be unset or 'n'.

bye, Roman

2002-07-22 01:05:51

by Keith Owens

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

On Sun, 21 Jul 2002 18:56:24 -0500 (CDT),
Kai Germaschewski <[email protected]> wrote:
>Makes sense to me. However, the CONFIG_ variables used in the Makefiles
>are never "n", they are "y", "m" or undefined.
>
>In Config.in scripts you have to cater for "n" or "", and I've seen
>various people on l-k carry this behavior into the Makefiles, but there
>it's unnecessary for all I can tell.

It is required if you ever want autoconfigure to work, that
distinguishes between "" (undefined) and "n" (explicitly turned off).
Forward planning.

2002-07-22 08:52:25

by Russell King

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

On Mon, Jul 22, 2002 at 10:51:43AM +0200, Andreas Schwab wrote:
> They do, see for example load_config_file in scripts/Menuconfig, or around
> line 556 in script/Configure.

What Roman is meaning is something like this:

if [ "$CONFIG_FOO" = "y" ]; then
choice ...
fi

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2002-07-22 09:21:22

by Roman Zippel

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

Hi,

On Mon, 22 Jul 2002, Russell King wrote:

> What Roman is meaning is something like this:
>
> if [ "$CONFIG_FOO" = "y" ]; then
> choice ...
> fi

BTW that's not the only problem, a symbol might be defined in
arch/foo/config.in, but not in arch/bar/config.in, so even xconfig doesn't
see everything.

bye, Roman

2002-07-22 08:48:38

by Andreas Schwab

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

Roman Zippel <[email protected]> writes:

|> Hi,
|>
|> On Mon, 22 Jul 2002, Russell King wrote:
|>
|> > Wouldn't it be better to fix the existing config tools to output "=n"
|> > instead of "# CONFIG_foo is not set" ? IIRC they do the translation
|> > back and forth internally anyway, so it should be just a matter of
|> > removing some code from the tools.
|>
|> This would mean, tristate symbols had four states instead of three. The
|> current shell based config systems simply don't see all symbols.

They do, see for example load_config_file in scripts/Menuconfig, or around
line 556 in script/Configure.

|> Depending on the configuration a symbol could be unset or 'n'.

A symbol is unset if it does not occur in .config at all. Having "#
CONFIG_foo is not set" in .config is completely the same as
"CONFIG_foo=n".

Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 N?rnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2002-07-22 14:26:20

by Kai Germaschewski

[permalink] [raw]
Subject: Re: [patch] 2.5.25 net/core/Makefile

On Mon, 22 Jul 2002, Russell King wrote:

> On Mon, Jul 22, 2002 at 11:08:41AM +1000, Keith Owens wrote:
> > It is required if you ever want autoconfigure to work, that
> > distinguishes between "" (undefined) and "n" (explicitly turned off).
> > Forward planning.
>
> Wouldn't it be better to fix the existing config tools to output "=n"
> instead of "# CONFIG_foo is not set" ? IIRC they do the translation
> back and forth internally anyway, so it should be just a matter of
> removing some code from the tools.

The point is, what would such a change buy us? It needs going through all
Makefiles, updating

ifdef CONFIG_XYZ

to

ifneq ($(CONFIG_XYZ),n)


(or ifeq ($(CONFIG_XYZ),y) when we now it's a bool)

Actually, now this won't handle the CONFIG_XYZ unset case, which may well
happen since a part of Config.in which would set or unset the symbols
may not even get sourced.

So we really have to use

ifneq ($(subst n,,$(CONFIG_XYZ),)

instead. That's ugly and doesn't have any advantage over what we have
now, AFAICS.

Inside the Config.in scripts it's annoying that you have to check against
"n" || "" (or ! ("y" || "m") ). The reasons for that lie, for all I can
tell, in the use of sh/bash for the original Configure script. In any
case, if this behavior is considered too annoying, it should be fixed in
the config system, but there's no reason to change the Makefile/.config
syntax, too.

(I think it may actually possible to fix it even within Configure, by just
always using <undef> instead of "n" - Configure needs to keep track of all
the encountered symbols anyway - but I didn't try. This would also
decrease the confusion, as then both the Makefiles and Config.in would
both use "y","m",<undef>.

W.r.t autoconfigure, I think that can easily be achieved using/extending
the existing .config format.

Just have

# CONFIG_FOO is not set
vs
# CONFIG_FOO=n

or even

<CONFIG_FOO not mentioned in .config>

vs the current

# CONFIG_FOO is not set

Should be nicely compatible with all existing tools.

--Kai