[PATCH] fix dependency generation

[PATCH] fix dependency generation

[Jan Beulich]

Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
the split config tree is built, but failed to also adjust fixdep
accordingly - if changing a config option from or to m, files
referencing the respective CONFIG_..._MODULE (but not the
corresponding CONFIG_...) didn't get rebuilt.

Once at it, also eliminate false dependencies due to use of
...CONFIG_... identifiers.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- linux-2.6.21-rc5/scripts/basic/fixdep.c	2007-02-04 19:44:54.000000000 +0100
+++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c	2007-03-29 11:11:10.000000000 +0200
@@ -29,8 +29,7 @@
  * option which is mentioned in any of the listed prequisites.
  *
  * To be exact, split-include populates a tree in include/config/,
- * e.g. include/config/his/driver.h, which contains the #define/#undef
- * for the CONFIG_HIS_DRIVER option.
+ * e.g. include/config/his/driver.h, consiting of empty files.
  *
  * So if the user changes his CONFIG_HIS_DRIVER option, only the objects
  * which depend on "include/linux/config/his/driver.h" will be rebuilt,
@@ -223,7 +222,7 @@ void use_config(char *m, int slen)
 void parse_config_file(char *map, size_t len)
 {
 	int *end = (int *) (map + len);
-	/* start at +1, so that p can never be < map */
+	/* start at +1, so that p can never be <= map */
 	int *m   = (int *) map + 1;
 	char *p, *q;
 
@@ -235,6 +234,8 @@ void parse_config_file(char *map, size_t
 		continue;
 	conf:
 		if (p > map + len - 7)
+			break;
+		if (isalnum(p[-1]) || p[-1] == '_')
 			continue;
 		if (memcmp(p, "CONFIG_", 7))
 			continue;
@@ -245,6 +246,8 @@ void parse_config_file(char *map, size_t
 		continue;
 
 	found:
+		if (!memcmp(q - 7, "_MODULE", 7))
+			q -= 7;
 		use_config(p+7, q-p-7);
 	}
 }

Re: [PATCH] fix dependency generation

[Randy Dunlap]

On Thu, 29 Mar 2007 10:27:14 +0100 Jan Beulich wrote:

> Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
> the split config tree is built, but failed to also adjust fixdep
> accordingly - if changing a config option from or to m, files
> referencing the respective CONFIG_..._MODULE (but not the
> corresponding CONFIG_...) didn't get rebuilt.
> 
> Once at it, also eliminate false dependencies due to use of
> ...CONFIG_... identifiers.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> --- linux-2.6.21-rc5/scripts/basic/fixdep.c	2007-02-04 19:44:54.000000000 +0100
> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c	2007-03-29 11:11:10.000000000 +0200
> @@ -29,8 +29,7 @@
>   * option which is mentioned in any of the listed prequisites.
>   *
>   * To be exact, split-include populates a tree in include/config/,
> - * e.g. include/config/his/driver.h, which contains the #define/#undef
> - * for the CONFIG_HIS_DRIVER option.

I don't see why you deleted the line above.

> + * e.g. include/config/his/driver.h, consiting of empty files.

                                        consisting

>   *
>   * So if the user changes his CONFIG_HIS_DRIVER option, only the objects
>   * which depend on "include/linux/config/his/driver.h" will be rebuilt,
> @@ -223,7 +222,7 @@ void use_config(char *m, int slen)
>  void parse_config_file(char *map, size_t len)
>  {
>  	int *end = (int *) (map + len);
> -	/* start at +1, so that p can never be < map */
> +	/* start at +1, so that p can never be <= map */
>  	int *m   = (int *) map + 1;
>  	char *p, *q;
>  
> @@ -235,6 +234,8 @@ void parse_config_file(char *map, size_t
>  		continue;
>  	conf:
>  		if (p > map + len - 7)
> +			break;
> +		if (isalnum(p[-1]) || p[-1] == '_')
>  			continue;
>  		if (memcmp(p, "CONFIG_", 7))
>  			continue;
> @@ -245,6 +246,8 @@ void parse_config_file(char *map, size_t
>  		continue;
>  
>  	found:
> +		if (!memcmp(q - 7, "_MODULE", 7))
> +			q -= 7;
>  		use_config(p+7, q-p-7);
>  	}
>  }


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] fix dependency generation

[Jan Beulich]

>>> Randy Dunlap <randy.dunlap@oracle.com> 29.03.07 17:39 >>>
>> --- linux-2.6.21-rc5/scripts/basic/fixdep.c	2007-02-04 19:44:54.000000000 +0100
>> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c	2007-03-29 11:11:10.000000000 +0200
>> @@ -29,8 +29,7 @@
>>   * option which is mentioned in any of the listed prequisites.
>>   *
>>   * To be exact, split-include populates a tree in include/config/,
>> - * e.g. include/config/his/driver.h, which contains the #define/#undef
>> - * for the CONFIG_HIS_DRIVER option.
>
>I don't see why you deleted the line above.

Because it is no longer true. These files are empty as of 2.6.18.

Jan

Re: [PATCH] fix dependency generation

[Randy Dunlap]

On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote:

> >>> Randy Dunlap <randy.dunlap@oracle.com> 29.03.07 17:39 >>>
> >> --- linux-2.6.21-rc5/scripts/basic/fixdep.c	2007-02-04 19:44:54.000000000 +0100
> >> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c	2007-03-29 11:11:10.000000000 +0200
> >> @@ -29,8 +29,7 @@
> >>   * option which is mentioned in any of the listed prequisites.
> >>   *
> >>   * To be exact, split-include populates a tree in include/config/,
> >> - * e.g. include/config/his/driver.h, which contains the #define/#undef
> >> - * for the CONFIG_HIS_DRIVER option.
> >
> >I don't see why you deleted the line above.
> 
> Because it is no longer true. These files are empty as of 2.6.18.

We seem to be talking about different lines above.  Yes, the files
are empty, but they are named based on the CONFIG_symbol name, which
is what I was trying to get at.  So how about a comment like this:

 * To be exact, split-include populates a tree in include/config/,
 * e.g., include/config/sysctl/syscall.h,
 * for the CONFIG_SYSCTL_SYSCALL option, when that option
 * is enabled (=y or =m).


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] fix dependency generation

[Jan Beulich]

>>> Randy Dunlap <randy.dunlap@oracle.com> 29.03.07 18:38 >>>
>On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote:
>
>> >>> Randy Dunlap <randy.dunlap@oracle.com> 29.03.07 17:39 >>>
>> >> --- linux-2.6.21-rc5/scripts/basic/fixdep.c	2007-02-04 19:44:54.000000000 +0100
>> >> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c	2007-03-29 11:11:10.000000000 +0200
>> >> @@ -29,8 +29,7 @@
>> >>   * option which is mentioned in any of the listed prequisites.
>> >>   *
>> >>   * To be exact, split-include populates a tree in include/config/,
>> >> - * e.g. include/config/his/driver.h, which contains the #define/#undef
>> >> - * for the CONFIG_HIS_DRIVER option.
>> >
>> >I don't see why you deleted the line above.
>> 
>> Because it is no longer true. These files are empty as of 2.6.18.
>
>We seem to be talking about different lines above.  Yes, the files
>are empty, but they are named based on the CONFIG_symbol name, which
>is what I was trying to get at.  So how about a comment like this:
>
> * To be exact, split-include populates a tree in include/config/,
> * e.g., include/config/sysctl/syscall.h,
> * for the CONFIG_SYSCTL_SYSCALL option, when that option
> * is enabled (=y or =m).

Shouldn't that then be '..., when that option is or ever was enabled
(=y or =m) since last cleaning the tree'?

Jan

Re: [PATCH] fix dependency generation

[Sam Ravnborg]

On Thu, Mar 29, 2007 at 10:27:14AM +0100, Jan Beulich wrote:
> Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
> the split config tree is built, but failed to also adjust fixdep
> accordingly - if changing a config option from or to m, files
> referencing the respective CONFIG_..._MODULE (but not the
> corresponding CONFIG_...) didn't get rebuilt.
Do you have a test case for this?
I want to play a little with this before I submit it.

> 
> Once at it, also eliminate false dependencies due to use of
> ...CONFIG_... identifiers.
But that will break UM - no??
See following note from fixdep:
 * Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend onto
 * CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not
 * fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as
 * UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h,
 * through arch/um/include/uml-config.h; this fixdep "bug" makes sure that
 * those files will have correct dependencies.

	Sam

> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> --- linux-2.6.21-rc5/scripts/basic/fixdep.c	2007-02-04 19:44:54.000000000 +0100
> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c	2007-03-29 11:11:10.000000000 +0200
> @@ -29,8 +29,7 @@
>   * option which is mentioned in any of the listed prequisites.
>   *
>   * To be exact, split-include populates a tree in include/config/,
> - * e.g. include/config/his/driver.h, which contains the #define/#undef
> - * for the CONFIG_HIS_DRIVER option.
> + * e.g. include/config/his/driver.h, consiting of empty files.
>   *
>   * So if the user changes his CONFIG_HIS_DRIVER option, only the objects
>   * which depend on "include/linux/config/his/driver.h" will be rebuilt,
> @@ -223,7 +222,7 @@ void use_config(char *m, int slen)
>  void parse_config_file(char *map, size_t len)
>  {
>  	int *end = (int *) (map + len);
> -	/* start at +1, so that p can never be < map */
> +	/* start at +1, so that p can never be <= map */
>  	int *m   = (int *) map + 1;
>  	char *p, *q;
>  
> @@ -235,6 +234,8 @@ void parse_config_file(char *map, size_t
>  		continue;
>  	conf:
>  		if (p > map + len - 7)
> +			break;
> +		if (isalnum(p[-1]) || p[-1] == '_')
>  			continue;
>  		if (memcmp(p, "CONFIG_", 7))
>  			continue;
> @@ -245,6 +246,8 @@ void parse_config_file(char *map, size_t
>  		continue;
>  
>  	found:
> +		if (!memcmp(q - 7, "_MODULE", 7))
> +			q -= 7;
>  		use_config(p+7, q-p-7);
>  	}
>  }
> 
> 

Re: [PATCH] fix dependency generation

[Randy Dunlap]

Jan Beulich wrote:
>>>> Randy Dunlap <randy.dunlap@oracle.com> 29.03.07 18:38 >>>
>> On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote:
>>
>>>>>> Randy Dunlap <randy.dunlap@oracle.com> 29.03.07 17:39 >>>
>>>>> --- linux-2.6.21-rc5/scripts/basic/fixdep.c	2007-02-04 19:44:54.000000000 +0100
>>>>> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c	2007-03-29 11:11:10.000000000 +0200
>>>>> @@ -29,8 +29,7 @@
>>>>>   * option which is mentioned in any of the listed prequisites.
>>>>>   *
>>>>>   * To be exact, split-include populates a tree in include/config/,
>>>>> - * e.g. include/config/his/driver.h, which contains the #define/#undef
>>>>> - * for the CONFIG_HIS_DRIVER option.
>>>> I don't see why you deleted the line above.
>>> Because it is no longer true. These files are empty as of 2.6.18.
>> We seem to be talking about different lines above.  Yes, the files
>> are empty, but they are named based on the CONFIG_symbol name, which
>> is what I was trying to get at.  So how about a comment like this:
>>
>> * To be exact, split-include populates a tree in include/config/,
>> * e.g., include/config/sysctl/syscall.h,
>> * for the CONFIG_SYSCTL_SYSCALL option, when that option
>> * is enabled (=y or =m).
> 
> Shouldn't that then be '..., when that option is or ever was enabled
> (=y or =m) since last cleaning the tree'?

Uh, I have no idea, but I guess that's OK with me.

-- 
~Randy

Re: [PATCH] fix dependency generation

[Jan Beulich]

>>> Sam Ravnborg <sam@ravnborg.org> 30.03.07 17:08 >>>
>On Thu, Mar 29, 2007 at 10:27:14AM +0100, Jan Beulich wrote:
>> Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
>> the split config tree is built, but failed to also adjust fixdep
>> accordingly - if changing a config option from or to m, files
>> referencing the respective CONFIG_..._MODULE (but not the
>> corresponding CONFIG_...) didn't get rebuilt.
>Do you have a test case for this?
>I want to play a little with this before I submit it.

On i386, set CONFIG_APM=y, build, then change it to m.

>> 
>> Once at it, also eliminate false dependencies due to use of
>> ...CONFIG_... identifiers.
>But that will break UM - no??
>See following note from fixdep:
> * Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend onto
> * CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not
> * fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as
> * UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h,
> * through arch/um/include/uml-config.h; this fixdep "bug" makes sure that
> * those files will have correct dependencies.

Hmm, didn't see this note. Then this might warrant special casing UML, but
penalizing all code due to this seems at least odd to me.

Jan

Re: [PATCH] fix dependency generation

[Jeff Dike]

On Fri, Mar 30, 2007 at 04:43:17PM +0100, Jan Beulich wrote:
> >But that will break UM - no??
> >See following note from fixdep:
> > * Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend onto
> > * CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not
> > * fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as
> > * UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h,
> > * through arch/um/include/uml-config.h; this fixdep "bug" makes sure that
> > * those files will have correct dependencies.
> 
> Hmm, didn't see this note. Then this might warrant special casing UML, but
> penalizing all code due to this seems at least odd to me.

If I understand this, I would think that special-casing UML_CONFIG_*
instead of *_CONFIG_* would be the way to go.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

Re: [PATCH] fix dependency generation

[Sam Ravnborg]

On Thu, Mar 29, 2007 at 10:27:14AM +0100, Jan Beulich wrote:
> Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
> the split config tree is built, but failed to also adjust fixdep
> accordingly - if changing a config option from or to m, files
> referencing the respective CONFIG_..._MODULE (but not the
> corresponding CONFIG_...) didn't get rebuilt.

The problem is that tristate symbol represent three values.
=n => CONFIG_SYMBOL is undefined
=y => CONFIG_SYMBOL is defined
=m => COMFIG_SYMBOL_MODULE is defined

The function split_config does not take into account the
different values and 'fixing' this in fixdep is wrong.
Because fixdep does not know if the variable is a tristate symbol or not
so it can either blindly remove _MODULE (your patch)
or each time it encounters _MODULE check for a symbol with and
without _MODULE.

The better fix is to teach the split_config function that
for tristate symbols two files shall be created in the include/config
hirachy. So for apm this gets:
include/config/apm.h
include/config/apm/module.h

This will make kconfig behave correct the day that someone add a config
symbol with a _MODULE suffix.

I will follow-up with two patches that implement the changes to split_config.
The first is a pure code refactoring preparing for the second patch.

Roman - please ack/nack these this since they touches kconfig backend.

	Sam


> 
> Once at it, also eliminate false dependencies due to use of
> ...CONFIG_... identifiers.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> --- linux-2.6.21-rc5/scripts/basic/fixdep.c	2007-02-04 19:44:54.000000000 +0100
> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c	2007-03-29 11:11:10.000000000 +0200
> @@ -29,8 +29,7 @@
>   * option which is mentioned in any of the listed prequisites.
>   *
>   * To be exact, split-include populates a tree in include/config/,
> - * e.g. include/config/his/driver.h, which contains the #define/#undef
> - * for the CONFIG_HIS_DRIVER option.
> + * e.g. include/config/his/driver.h, consiting of empty files.
>   *
>   * So if the user changes his CONFIG_HIS_DRIVER option, only the objects
>   * which depend on "include/linux/config/his/driver.h" will be rebuilt,
> @@ -223,7 +222,7 @@ void use_config(char *m, int slen)
>  void parse_config_file(char *map, size_t len)
>  {
>  	int *end = (int *) (map + len);
> -	/* start at +1, so that p can never be < map */
> +	/* start at +1, so that p can never be <= map */
>  	int *m   = (int *) map + 1;
>  	char *p, *q;
>  
> @@ -235,6 +234,8 @@ void parse_config_file(char *map, size_t
>  		continue;
>  	conf:
>  		if (p > map + len - 7)
> +			break;
> +		if (isalnum(p[-1]) || p[-1] == '_')
>  			continue;
>  		if (memcmp(p, "CONFIG_", 7))
>  			continue;
> @@ -245,6 +246,8 @@ void parse_config_file(char *map, size_t
>  		continue;
>  
>  	found:
> +		if (!memcmp(q - 7, "_MODULE", 7))
> +			q -= 7;
>  		use_config(p+7, q-p-7);
>  	}
>  }
> 
> 

[PATCH 1/2] kconfig: factor out code in conf_spilt_config

[Sam Ravnborg]

>From e9fcc3bf8d1c71df1ae650d5291c5d4b15d71656 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <sam@neptun.ravnborg.org>
Date: Sat, 31 Mar 2007 09:15:12 +0200
Subject: [PATCH] kconfig: factor out code in conf_spilt_config

This patch simply factor out code and
do not introduce any functional changes.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
 scripts/kconfig/confdata.c |   75 ++++++++++++++++++++++++--------------------
 1 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 664fe29..ff6b39b 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -533,13 +533,48 @@ int conf_write(const char *name)
 	return 0;
 }
 
+/* Touch the file specified (adding .h to the name) */
+static int touch_file(const char *file)
+{
+	struct stat sb;
+	int fd;
+	char *d;
+	char name[128];
+
+	strcpy(name, file);
+	strcat(name, ".h");
+
+	/* Open existing file */
+	fd = open(name, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+	if (fd == -1) {
+		if (errno != ENOENT)
+			return 1;
+		/*
+		 * Create directory components,
+		 * unless they exist already.
+		 */
+		d = name;
+		while ((d = strchr(d, '/'))) {
+			*d = 0;
+			if (stat(name, &sb) && mkdir(name, 0755))
+				return 1;
+			*d++ = '/';
+		}
+		/* Directories created, now create file. */
+		fd = open(name, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+		if (fd == -1)
+			return 1;
+	}
+	close(fd);
+	return 0;
+}
+
 int conf_split_config(void)
 {
 	char *name, path[128];
 	char *s, *d, c;
 	struct symbol *sym;
-	struct stat sb;
-	int res, i, fd;
+	int res, i;
 
 	name = getenv("KCONFIG_AUTOCONFIG");
 	if (!name)
@@ -601,45 +636,17 @@ int conf_split_config(void)
 		 *	different from 'no').
 		 */
 
-		/* Replace all '_' and append ".h" */
+		/* Replace all '_' with '/' */
 		s = sym->name;
 		d = path;
 		while ((c = *s++)) {
 			c = tolower(c);
 			*d++ = (c == '_') ? '/' : c;
 		}
-		strcpy(d, ".h");
-
-		/* Assume directory path already exists. */
-		fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
-		if (fd == -1) {
-			if (errno != ENOENT) {
-				res = 1;
-				break;
-			}
-			/*
-			 * Create directory components,
-			 * unless they exist already.
-			 */
-			d = path;
-			while ((d = strchr(d, '/'))) {
-				*d = 0;
-				if (stat(path, &sb) && mkdir(path, 0755)) {
-					res = 1;
-					goto out;
-				}
-				*d++ = '/';
-			}
-			/* Try it again. */
-			fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
-			if (fd == -1) {
-				res = 1;
-				break;
-			}
-		}
-		close(fd);
+		*d = '\0';
+		if (touch_file(path))
+			return 1;
 	}
-out:
 	if (chdir("../.."))
 		return 1;
 
-- 
1.5.1.rc3.gaa453

[PATCH 2/2] kconfig/kbuild: fix dependency problem

[Sam Ravnborg]

>From bbc89026f3e5d9e437ce4cd26d3013fe226103e2 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <sam@neptun.ravnborg.org>
Date: Sat, 31 Mar 2007 09:34:46 +0200
Subject: [PATCH] kconfig/kbuild: fix dependency problem

Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
the split config tree is built, but failed to also adjust fixdep
accordingly - if changing a config option from or to m, files
referencing the respective CONFIG_..._MODULE (but not the
corresponding CONFIG_...) didn't get rebuilt.

This happens because tristate symbol has three values represented
by different CONFIG_ symbols:

=n => CONFIG_SYMBOL undefined
=y => CONFIG_SYMBOL equals 1
=m => CONFIG_SYMBOL_MODULE equals 1

But conf_split_config did not support the _MODULE syntax
and therefore no include/config/symbol/module.h file was
generated/touched when changing a symbol to/from m.
Thus make did nt pick up the change and rebuild failed.

This patch teaches conf_split_config to support the
_MODULE variant.

This fixes a problem reported by Randy Dunlap <randy.dunlap@oracle.com>.
arch/i386/kernel/apm.o revealed this bug.
Original fix was posted by:  "Jan Beulich" <jbeulich@novell.com>
which inspired this better fix.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
 scripts/kconfig/confdata.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index ff6b39b..4137961 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -646,6 +646,12 @@ int conf_split_config(void)
 		*d = '\0';
 		if (touch_file(path))
 			return 1;
+		/* For tristate symbols we need to touch symbol/module.h too */
+		if (sym->type == S_TRISTATE) {
+			strcat(path, "/module");
+			if (touch_file(path))
+				return 1;
+		}
 	}
 	if (chdir("../.."))
 		return 1;
-- 
1.5.1.rc3.gaa453

Re: [PATCH] fix dependency generation

[Roman Zippel]

Hi,

On Sat, 31 Mar 2007, Sam Ravnborg wrote:

> The problem is that tristate symbol represent three values.
> =n => CONFIG_SYMBOL is undefined
> =y => CONFIG_SYMBOL is defined
> =m => COMFIG_SYMBOL_MODULE is defined
> 
> The function split_config does not take into account the
> different values and 'fixing' this in fixdep is wrong.
> Because fixdep does not know if the variable is a tristate symbol or not
> so it can either blindly remove _MODULE (your patch)
> or each time it encounters _MODULE check for a symbol with and
> without _MODULE.

What really matters is that CONFIG_SYMBOL changed, one could optimize for 
the COMFIG_SYMBOL_MODULE case, but I don't think it's worth it, especially 
...

> The better fix is to teach the split_config function that
> for tristate symbols two files shall be created in the include/config
> hirachy. So for apm this gets:
> include/config/apm.h
> include/config/apm/module.h

if it requires thousands of new inodes for a feature which should be 
rarely used.

> This will make kconfig behave correct the day that someone add a config
> symbol with a _MODULE suffix.

I'd rather reserve that namespace, if it allows for the simpler version to 
just map all symbols to the basic config symbol name.

bye, Roman

Re: [PATCH] fix dependency generation

[Sam Ravnborg]

On Sat, Mar 31, 2007 at 06:11:36PM +0200, Roman Zippel wrote:
> Hi,
> 
> On Sat, 31 Mar 2007, Sam Ravnborg wrote:
> 
> > The problem is that tristate symbol represent three values.
> > =n => CONFIG_SYMBOL is undefined
> > =y => CONFIG_SYMBOL is defined
> > =m => COMFIG_SYMBOL_MODULE is defined
> > 
> > The function split_config does not take into account the
> > different values and 'fixing' this in fixdep is wrong.
> > Because fixdep does not know if the variable is a tristate symbol or not
> > so it can either blindly remove _MODULE (your patch)
> > or each time it encounters _MODULE check for a symbol with and
> > without _MODULE.
> 
> What really matters is that CONFIG_SYMBOL changed, one could optimize for 
> the COMFIG_SYMBOL_MODULE case, but I don't think it's worth it, especially 
> ...
> 
> > The better fix is to teach the split_config function that
> > for tristate symbols two files shall be created in the include/config
> > hirachy. So for apm this gets:
> > include/config/apm.h
> > include/config/apm/module.h
> 
> if it requires thousands of new inodes for a feature which should be 
> rarely used.
> 
> > This will make kconfig behave correct the day that someone add a config
> > symbol with a _MODULE suffix.
> 
> I'd rather reserve that namespace, if it allows for the simpler version to 
> just map all symbols to the basic config symbol name.

OK, I have forwarded Jan's patch for inclusion (the bug-fix part).

	Sam

> 
> bye, Roman