NetBSD Problem Report #38280

From tsutsui@ceres.dti.ne.jp  Sat Mar 22 20:28:58 2008
Return-Path: <tsutsui@ceres.dti.ne.jp>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 350B663B863
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 22 Mar 2008 20:28:58 +0000 (UTC)
Message-Id: <200803222028.m2MKStSY021735@mirage.ceres.dti.ne.jp>
Date: Sun, 23 Mar 2008 05:28:55 +0900 (JST)
From: tsutsui@ceres.dti.ne.jp
Reply-To: tsutsui@ceres.dti.ne.jp
To: gnats-bugs@gnats.NetBSD.org
Cc: tsutsui@ceres.dti.ne.jp
Subject: config(1) should emit devsw.h for devsw declarations in devsw.c
X-Send-Pr-Version: 3.95

>Number:         38280
>Category:       toolchain
>Synopsis:       config(1) should emit devsw.h for devsw declarations in devsw.c
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    toolchain-manager
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sat Mar 22 20:30:00 +0000 2008
>Last-Modified:  Sat Mar 22 22:20:01 +0000 2008
>Originator:     Izumi Tsutsui
>Release:        NetBSD 4.99.55
>Organization:
>Environment:
Machine independent
>Description:
Currently many sources and drivers have declarations like
"extern const struct cdevsw foo_cdevsw;"
but it should be autogenerated by config(1) as well as ioconf.h.

>How-To-Repeat:
Code inspection.

>Fix:
Apply the attached patch.

Note this also contains the following changes:
- use __arraycount()
- put "DO NOT EDIT" header into locators.h and ioconf.h too

Index: defs.h
===================================================================
RCS file: /cvsroot/src/usr.bin/config/defs.h,v
retrieving revision 1.22
diff -u -r1.22 defs.h
--- defs.h	12 Dec 2007 00:03:33 -0000	1.22
+++ defs.h	22 Mar 2008 20:16:27 -0000
@@ -98,7 +98,7 @@
  * The next two lines define the current version of the config(1) binary,
  * and the minimum version of the configuration files it supports.
  */
-#define CONFIG_VERSION		20071109
+#define CONFIG_VERSION		20080323
 #define CONFIG_MINVERSION	0

 /*
Index: mkdevsw.c
===================================================================
RCS file: /cvsroot/src/usr.bin/config/mkdevsw.c,v
retrieving revision 1.5
diff -u -r1.5 mkdevsw.c
--- mkdevsw.c	12 Dec 2007 00:03:33 -0000	1.5
+++ mkdevsw.c	22 Mar 2008 20:16:27 -0000
@@ -91,8 +91,8 @@

 	fputs("#include <sys/param.h>\n"
 		  "#include <sys/conf.h>\n"
-		  "\n#define\tDEVSW_ARRAY_SIZE(x)\t"
-		  "(sizeof((x))/sizeof((x)[0]))\n", fp);
+		  "#include \"devsw.h\"\n"
+		  "\n#define\tDEVSW_ARRAY_SIZE(x)\t__arraycount(x)\n", fp);
 }

 /*
@@ -107,15 +107,6 @@

 	fputs("\n/* device switch table for block device */\n", fp);

-	for (i = 0 ; i <= maxbdevm ; i++) {
-		(void)snprintf(mstr, sizeof(mstr), "%d", i);
-		if ((dm = ht_lookup(bdevmtab, intern(mstr))) == NULL)
-			continue;
-
-		fprintf(fp, "extern const struct bdevsw %s_bdevsw;\n",
-			    dm->dm_name);
-	}
-
 	fputs("\nconst struct bdevsw *bdevsw0[] = {\n", fp);

 	for (i = 0 ; i <= maxbdevm ; i++) {
@@ -134,15 +125,6 @@

 	fputs("\n/* device switch table for character device */\n", fp);

-	for (i = 0 ; i <= maxcdevm ; i++) {
-		(void)snprintf(mstr, sizeof(mstr), "%d", i);
-		if ((dm = ht_lookup(cdevmtab, intern(mstr))) == NULL)
-			continue;
-
-		fprintf(fp, "extern const struct cdevsw %s_cdevsw;\n",
-			    dm->dm_name);
-	}
-
 	fputs("\nconst struct cdevsw *cdevsw0[] = {\n", fp);

 	for (i = 0 ; i <= maxcdevm ; i++) {
Index: mkheaders.c
===================================================================
RCS file: /cvsroot/src/usr.bin/config/mkheaders.c,v
retrieving revision 1.13
diff -u -r1.13 mkheaders.c
--- mkheaders.c	9 Nov 2007 05:21:30 -0000	1.13
+++ mkheaders.c	22 Mar 2008 20:16:27 -0000
@@ -61,6 +61,7 @@
 static int emitlocs(void);
 static int emitopts(void);
 static int emitioconfh(void);
+static int emitdevswh(void);
 static int emittime(void);
 static int herr(const char *, const char *, FILE *);
 static int defopts_print(const char *, void *, void *);
@@ -95,7 +96,8 @@
 			return (1);
 	}

-	if (emitopts() || emitlocs() || emitioconfh() || emittime())
+	if (emitopts() || emitlocs() || emitioconfh() || emitdevswh() ||
+	    emittime())
 		return (1);

 	return (0);
@@ -354,6 +356,8 @@
 	if ((tfp = fopen(tfname, "w")) == NULL)
 		return (herr("open", tfname, NULL));

+	autogen_comment(tfp, "locators.h");
+
 	rval = ht_enumerate(attrtab, locators_print, tfp);

 	fflush(tfp);
@@ -381,6 +385,8 @@
 	if ((tfp = fopen(tfname, "w")) == NULL)
 		return (herr("open", tfname, NULL));

+	autogen_comment(tfp, "ioconf.h");
+
 	TAILQ_FOREACH(d, &allbases, d_next) {
 		if (!devbase_has_instances(d, WILD))
 			continue;
@@ -398,6 +404,68 @@
 }

 /*
+ * Build the "devsw.h" file with extern declarations for all configured
+ * bdevsw and cdevsw.
+ */
+static int
+emitdevswh(void)
+{
+	const char *tfname;
+	FILE *tfp;
+	struct devm *dm;
+	char mstr[16];
+	int i;
+
+	tfname = "tmp_devsw.h";
+	if ((tfp = fopen(tfname, "w")) == NULL)
+		return (herr("open", tfname, NULL));
+
+	autogen_comment(tfp, "devsw.h");
+
+	for (i = 0 ; i <= maxbdevm ; i++) {
+		(void)snprintf(mstr, sizeof(mstr), "%d", i);
+		if ((dm = ht_lookup(bdevmtab, intern(mstr))) == NULL)
+			continue;
+
+		fprintf(tfp, "extern const struct bdevsw %s_bdevsw;\n",
+		    dm->dm_name);
+	}
+	fprintf(tfp, "\n");
+
+	fprintf(tfp, "extern const struct bdevsw **bdevsw, *bdevsw0[];\n");
+	fprintf(tfp, "extern const int sys_bdevsws;\n");
+	fprintf(tfp, "extern int max_bdevsws;\n");
+	fprintf(tfp, "\n");
+
+	for (i = 0 ; i <= maxcdevm ; i++) {
+		(void)snprintf(mstr, sizeof(mstr), "%d", i);
+		if ((dm = ht_lookup(cdevmtab, intern(mstr))) == NULL)
+			continue;
+
+		fprintf(tfp, "extern const struct cdevsw %s_cdevsw;\n",
+		    dm->dm_name);
+	}
+	fprintf(tfp, "\n");
+
+	fprintf(tfp, "extern const struct cdevsw **cdevsw, *cdevsw0[];\n");
+	fprintf(tfp, "extern const int sys_cdevsws;\n");
+	fprintf(tfp, "extern int max_cdevsws;\n");
+	fprintf(tfp, "\n");
+
+	fprintf(tfp, "extern struct devsw_conv *devsw_conv;\n");
+	fprintf(tfp, "extern struct devsw_conv devsw_conv0[];\n");
+	fprintf(tfp, "extern int max_devsw_convs;\n");
+			
+	fflush(tfp);
+	if (ferror(tfp))
+		return herr("writ", tfname, tfp);
+
+	if (fclose(tfp) == EOF)
+		return (herr("clos", tfname, NULL));
+
+	return (moveifchanged(tfname, "devsw.h"));
+}
+/*
  * Make a file that config_time.h can use as a source, if required.
  */
 static int

>Audit-Trail:
From: Quentin Garnier <cube@cubidou.net>
To: gnats-bugs@NetBSD.org
Cc: tsutsui@ceres.dti.ne.jp
Subject: Re: toolchain/38280: config(1) should emit devsw.h for devsw declarations in devsw.c
Date: Sat, 22 Mar 2008 22:49:04 +0100

 --J/F6JaM/TgkXbavH
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 Content-Transfer-Encoding: quoted-printable

 On Sat, Mar 22, 2008 at 08:30:01PM +0000, tsutsui@ceres.dti.ne.jp wrote:
 > >Number:         38280
 > >Category:       toolchain
 > >Synopsis:       config(1) should emit devsw.h for devsw declarations in =
 devsw.c
 > >Confidential:   no
 > >Severity:       non-critical
 > >Priority:       low
 > >Responsible:    toolchain-manager
 > >State:          open
 > >Class:          change-request
 > >Submitter-Id:   net
 > >Arrival-Date:   Sat Mar 22 20:30:00 +0000 2008
 > >Originator:     Izumi Tsutsui
 > >Release:        NetBSD 4.99.55
 > >Organization:
 > >Environment:
 > Machine independent
 > >Description:
 > Currently many sources and drivers have declarations like
 > "extern const struct cdevsw foo_cdevsw;"

 Wouldn't it be better to fix drivers to use devsw_name2chr() and
 devsw_name2blk()?  Directly referencing something like that gives me
 an ewww feeling.

 --=20
 Quentin Garnier - cube@cubidou.net - cube@NetBSD.org
 "See the look on my face from staying too long in one place
 [...] every time the morning breaks I know I'm closer to falling"
 KT Tunstall, Saving My Face, Drastic Fantastic, 2007.

 --J/F6JaM/TgkXbavH
 Content-Type: application/pgp-signature
 Content-Disposition: inline

 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.4.6 (NetBSD)

 iQEVAwUBR+V+0NgoQloHrPnoAQLbjQgAlyPqUhdbNBreN8JJU/M1h+kZIR3VkEyd
 UWp1zKevaWsIyyK7M+d7I8T3w9vqDCkDH+c/7O8st6hFxcCqon0qqimw2O1klA/O
 FKSUguKK3dMZBGJO7WqlVDkliKMRGW4Q7MdzeILV685ANzsts6dXTLZiPb4SY/iL
 9z8BmgR8faAOBhogTSMBWfPM4fUMV2aTDEOmvf3Tp/raq8CVEdIrrHo+k52RLDlN
 F8HYNagFcord78XbVtKbIncr4LXObh7gnNJdG9BpUNoILPoUgw0XTTx1XtPFjzLv
 JT2KwbAxc5Zs66J7WaxI1UrXzPD26HuehfrVVoZG2zPCTfhXlKEf5Q==
 =PY9A
 -----END PGP SIGNATURE-----

 --J/F6JaM/TgkXbavH--

From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: cube@cubidou.net
Cc: gnats-bugs@NetBSD.org, tsutsui@ceres.dti.ne.jp
Subject: Re: toolchain/38280: config(1) should emit devsw.h for devsw declarations
	 in devsw.c
Date: Sun, 23 Mar 2008 07:14:55 +0900

 > Wouldn't it be better to fix drivers to use devsw_name2chr() and
 > devsw_name2blk()?  Directly referencing something like that gives me
 > an ewww feeling.

 Main usage of it is
 	makedev(cdevsw_lookup_major(&foo_cdevsw));
 in console cnattach() code.

 Should we also use devsw_name2chr() there?

 BTW, is mutex(9) (which is used in devsw_name2chr())
 valid at the point where consinit() is called first?
 ---
 Izumi Tsutsui

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.