Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 14 Jan 2010 10:56:22 -0700 (MST)
From:      "M. Warner Losh" <imp@bsdimp.com>
To:        rwatson@freebsd.org
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, dougb@freebsd.org, src-committers@freebsd.org, freebsd-arch@freebsd.org
Subject:   Re: INCLUDE_CONFIG_FILE in GENERIC
Message-ID:  <20100114.105622.457034909117828677.imp@bsdimp.com>
In-Reply-To: <20100114.102142.328914705071816274.imp@bsdimp.com>
References:  <4B4E1586.7090102@FreeBSD.org> <alpine.BSF.2.00.1001141652140.49545@fledge.watson.org> <20100114.102142.328914705071816274.imp@bsdimp.com>

next in thread | previous in thread | raw e-mail | index | archive | help
----Next_Part(Thu_Jan_14_10_56_22_2010_136)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

In message: <20100114.102142.328914705071816274.imp@bsdimp.com>
            "M. Warner Losh" <imp@bsdimp.com> writes:
: Personally, I'd rather see us have two different options here, as
: suggested by others, so that it is clear and self-contained.  I'd vote
: for INCLUDE_CONFIG for today's behavior without -C, and
: INCLUDE_CONFIG_FILE for config -C.  -C then becomes a nop and the man
: page becomes correct.  We can then put a single line:
: 
: options 	INCLUDE_CONFIG_FILE    	 # include this config file in kernel
: 
: and be done with it in head.

Consider the following patches, and the following lines in GENERIC:

options 	INCLUDE_CONFIG_FILE    	 # include this config file in kernel
#options 	INCLUDE_CONFIG    	 # processed config, without comments

: For MFC, having the extra comment lines isn't so horrible as an
: explicitly granted exception to the rule of not doing that so others
: won't be tempted to do it too.

That was worded badly.  "I'm OK with merging the extra comments, so
long as it is clear to everybody that this is done as a special case
exception to the general rule that we don't do this."

Finally, I'd like to appologize to Doug for hijacking his simple
change and using it as an engine to make things better for the
project.

Warner

P.S.  I've also placed a copy of the diff at
http://people.freebsd.org/~imp/config.diff

----Next_Part(Thu_Jan_14_10_56_22_2010_136)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="config.diff"

Index: sys/kern/kern_mib.c
===================================================================
--- sys/kern/kern_mib.c	(revision 202102)
+++ sys/kern/kern_mib.c	(working copy)
@@ -351,7 +351,7 @@
     CTLTYPE_INT|CTLFLAG_RW|CTLFLAG_PRISON, 0, 0, sysctl_kern_securelvl,
     "I", "Current secure level");
 
-#ifdef INCLUDE_CONFIG_FILE
+#if defined(INCLUDE_CONFIG_FILE) || defined(INCLUDE_CONFIG)
 /* Actual kernel configuration options. */
 extern char kernconfstring[];
 
Index: usr.sbin/config/kernconf.tmpl
===================================================================
--- usr.sbin/config/kernconf.tmpl	(revision 202102)
+++ usr.sbin/config/kernconf.tmpl	(working copy)
@@ -5,13 +5,19 @@
  * $FreeBSD$
  */
 #include "opt_config.h"
-#ifdef INCLUDE_CONFIG_FILE
 
 /*
- * For !INCLUDE_CONFIG_FILE case, you should look at kern_mib.c. This is
- * where kernconfstring is defined then.
+ * If INCLUDE_CONFIG_FILE is defined, then we include the config file
+ * verbatim (and that's the only config file we include).  Otherwise, if
+ * INCLUDE_CONFIG is defined, we include it.  Otherwise, we include nothing
+ * at all.
  */
+#ifdef INCLUDE_CONFIG_FILE
 const char kernconfstring[] __attribute__ ((section("kern_conf"))) =
 "%%KERNCONFFILE%%";
-
+#else /* INCLUDE_CONFIG_FILE */
+#ifdef INCLUDE_CONFIG
+const char kernconfstring[] __attribute__ ((section("kern_conf"))) =
+"%%KERNCONF%%";
+#endif /* INCLUDE_CONFIG */
 #endif /* INCLUDE_CONFIG_FILE */
Index: usr.sbin/config/main.c
===================================================================
--- usr.sbin/config/main.c	(revision 202102)
+++ usr.sbin/config/main.c	(working copy)
@@ -79,12 +79,6 @@
 int	found_defaults;
 int	incignore;
 
-/*
- * Preserve old behaviour in INCLUDE_CONFIG_FILE handling (files are included
- * literally).
- */
-int	filebased = 0;
-
 static void configfile(void);
 static void get_srcdir(void);
 static void usage(void);
@@ -114,7 +108,6 @@
 	while ((ch = getopt(argc, argv, "Cd:gpVx:")) != -1)
 		switch (ch) {
 		case 'C':
-			filebased = 1;
 			break;
 		case 'd':
 			if (*destdir == '\0')
@@ -489,12 +482,18 @@
 	}
 }
 
+static int
+matches(const char *s1, const char *s2)
+{
+	return (strncmp(s1, s2, strlen(s2)) == 0);
+}
+
 static void
 configfile(void)
 {
 	FILE *fo;
-	struct sbuf *sb;
-	char *p;
+	struct sbuf *sb1, *sb2;
+	char *p, *walker;
 
 	/* Add main configuration file to the list of files to be included */
 	cfgfile_add(PREFIX);
@@ -502,30 +501,30 @@
 	fo = fopen(p, "w");
 	if (!fo)
 		err(2, "%s", p);
-	sb = sbuf_new(NULL, NULL, 2048, SBUF_AUTOEXTEND);
-	assert(sb != NULL);
-	sbuf_clear(sb);
-	if (filebased) {
-		/* Is needed, can be used for backward compatibility. */
-		configfile_filebased(sb);
-	} else {
-		configfile_dynamic(sb);
+	sb1 = sbuf_new(NULL, NULL, 2048, SBUF_AUTOEXTEND);
+	assert(sb1 != NULL);
+	sb2 = sbuf_new(NULL, NULL, 2048, SBUF_AUTOEXTEND);
+	assert(sb2 != NULL);
+	sbuf_clear(sb1);
+	configfile_filebased(sb1);
+	sbuf_finish(sb1);
+	sbuf_clear(sb2);
+	configfile_dynamic(sb2);
+	sbuf_finish(sb2);
+	walker = kernconfstr;
+	while (*walker) {
+		if (matches(walker, KERNCONFFILETAG)) {
+			fprintf(fo, "%s", sbuf_data(sb1));
+			walker += strlen(KERNCONFFILETAG);
+		} else if (matches(walker, KERNCONFTAG)) {
+			fprintf(fo, "%s", sbuf_data(sb2));
+			walker += strlen(KERNCONFTAG);
+		} else {
+			fputc(*walker++, fo);
+		}
 	}
-	sbuf_finish(sb);
-	/* 
-	 * We print first part of the tamplate, replace our tag with
-	 * configuration files content and later continue writing our
-	 * template.
-	 */
-	p = strstr(kernconfstr, KERNCONFTAG);
-	if (p == NULL)
-		errx(EXIT_FAILURE, "Something went terribly wrong!");
-	*p = '\0';
-	fprintf(fo, "%s", kernconfstr);
-	fprintf(fo, "%s", sbuf_data(sb));
-	p += strlen(KERNCONFTAG);
-	fprintf(fo, "%s", p);
-	sbuf_delete(sb);
+	sbuf_delete(sb1);
+	sbuf_delete(sb2);
 	fclose(fo);
 	moveifchanged(path("config.c.new"), path("config.c"));
 	cfgfile_removeall();
Index: usr.sbin/config/config.8
===================================================================
--- usr.sbin/config/config.8	(revision 202102)
+++ usr.sbin/config/config.8	(working copy)
@@ -65,9 +65,6 @@
 .Nm
 version number.
 .It Fl C
-If the INCLUDE_CONFIG_FILE is present in a configuration file,
-kernel image will contain full configuration files included
-literally (preserving comments).
 This flag is kept for backward compatibility.
 .It Fl d Ar destdir
 Use
@@ -83,7 +80,9 @@
 .It Fl x Ar kernel
 Print kernel configuration file embedded into a kernel
 file.
-This option makes sense only if 
+This option makes sense only if either 
+.Cd "options INCLUDE_CONFIG"
+or 
 .Cd "options INCLUDE_CONFIG_FILE"
 entry was present in your configuration file.
 .It Fl p
@@ -150,6 +149,17 @@
 should be run again.
 Attempts to compile a system that had configuration errors
 are likely to fail.
+.Pp
+The kernel config file can be embedded into the kernel binary in one of
+two ways.
+First, a verbatim copy of the kernel config text file, without any of the
+included config files, can be included using 
+.Cd "options INCLUDE_CONFIG_FILE"
+in the config file.
+Second, a copy of the fully process kernel config file, without any comments
+at all, can be included using
+.Cd "options INCLUDE_CONFIG"
+in the config file.
 .Sh DEBUG KERNELS
 Traditional
 .Bx
Index: usr.sbin/config/config.h
===================================================================
--- usr.sbin/config/config.h	(revision 202102)
+++ usr.sbin/config/config.h	(working copy)
@@ -142,10 +142,11 @@
 STAILQ_HEAD(hint_head, hint) hints;
 
 /*
- * Tag present in the kernelconf.tmlp template file. It's mandatory for those
- * two strings to be the same. Otherwise you'll get into trouble.
+ * Tags expanded in the kernconf.tmpl file.  KERNCONFTAG is for the processed
+ * kernel file, while KERNCONFFILE is for the verbatim file.
  */
-#define	KERNCONFTAG	"%%KERNCONFFILE%%"
+#define	KERNCONFTAG	"%%KERNCONF%%"
+#define	KERNCONFFILETAG	"%%KERNCONFFILE%%"
 
 /*
  * Faked option to note, that the configuration file has been taken from the

----Next_Part(Thu_Jan_14_10_56_22_2010_136)----



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100114.105622.457034909117828677.imp>