Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 8 Oct 2003 15:07:22 -0400 (EDT)
From:      "Adam C. Migus" <adam@migus.org>
To:        "Bruce Evans" <bde@zeta.org.au>
Cc:        arch@freebsd.org
Subject:   Re: sys/conf/DEFAULT[S]
Message-ID:  <51574.204.254.155.35.1065640042.squirrel@mail.migus.org>
In-Reply-To: <20031008212302.T4729@gamplex.bde.org>
References:  <XFMail.20030924170342.jhb@FreeBSD.org>      <20030925092319.H5418@gamplex.bde.org><49939.204.254.155.35.1064593320.squirrel@mail.migus.org> <20030927080420.N18558@gamplex.bde.org> <49955.192.168.4.2.1065074430.squirrel@mail.migus.org> <20031008212302.T4729@gamplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help

Bruce Evans said:
> On Thu, 2 Oct 2003, Adam C. Migus wrote:
>
>> > On Fri, 26 Sep 2003, Adam C. Migus wrote:
>>
>> This patch works for me, please let me know if there's any
>> problems
>> with it or you'd like a PR.
>
> % ====
> //depot/user/amigus/freebsd-amigus/src/usr.sbin/config/config.y#1 -
> /src/p4/user/amigus/perforce.freebsd.org/freebsd-amigus/usr.sbin/config/config.y
> ====
> % @@ -118,6 +118,8 @@
> %  		|
> %  	Config_spec SEMICOLON
> %  		|
> % +	Include
> % +		|
> %  	SEMICOLON
> %  		|
> %  	error SEMICOLON
> % @@ -164,9 +166,7 @@
> %  	      = {
> %  		      hints = $2;
> %  		      hintmode = 1;
> % -	        } |
> % -	INCLUDE ID
> % -	      = { include($2, 0); };
> % +	        };
> %
> %  System_spec:
> %  	CONFIG System_id System_parameter_list
> % @@ -265,6 +265,11 @@
> %  		rmdev($2);
> %  		} ;
> %
> % +Include:
> % +	INCLUDE ID
> % +	      = { include($2, 0); };
> % +
> % +
> %  %%
> %
> %  void
>
> I found 1 problem with this: it doesn't require SEMICOLON after
> "INCLUDE ID", so parsing resumes after "ID" when the input stream
> is switched back to the includer.  You can say things like
>
> 	include FOO		device foo
>
> and then the "device foo" directive actually works.  This can be
> considered a feature, but the following is not:
>
> 	include GENERIC.local
>
> at the end of GENERIC should be a syntax error, but it actually
> matches ID = GENERIC and includes GENERIC recursively, which gives
> a confusing error message.  There would be a syntax error on
> switching
> back but config aborts before then.  (Filenames other than single
> identifiers must be quoted to avoid problems like this.  This is not
> very obvious since config was changed to not require quotes in most
> contexts.)
>
> Stefan's version requires SEMICOLON:
>
> % Index: src/usr.sbin/config/config.y
> %
> ===================================================================
> % RCS file: /usr/home/ncvs/src/usr.sbin/config/config.y,v
> % retrieving revision 1.61
> % diff -u -r1.61 config.y
> % --- src/usr.sbin/config/config.y	6 Jul 2003 02:00:52 -0000	1.61
> % +++ src/usr.sbin/config/config.y	27 Sep 2003 10:39:13 -0000
> % @@ -118,6 +118,9 @@
> %  		|
> %  	Config_spec SEMICOLON
> %  		|
> % +	INCLUDE ID SEMICOLON
> % +	      = { include($2, 0); };
> % +		|
> %  	SEMICOLON
> %  		|
> %  	error SEMICOLON
> % @@ -164,9 +167,7 @@
> %  	      = {
> %  		      hints = $2;
> %  		      hintmode = 1;
> % -	        } |
> % -	INCLUDE ID
> % -	      = { include($2, 0); };
> % +	        }
> %
> %  System_spec:
> %  	CONFIG System_id System_parameter_list
>
> I lost your examples showing that this doesn't quite work.  What is
> the
> problem with it?
>
> Another old bug is that using "include" defeats the point of the
> INCLUDE_CONFIG_FILE option (since included files aren't expanded).
>
> Bruce
>

Bruce,
Ah yes, my bad.  It was a patching error.  Specifically my error in
not applying the patch but rather reading and carelessly assuming
that Stefan had simply appended a SEMICOLON to the existing code, as
his patch.  :-)

When I correctly apply Stefan's patch it fixes this problem however
with Stefan's patch the code is essentially equivalent to mine, so,
any caveats except those involving the absent SEMICOLON, still
apply.  For example the recursive include case you pointed out.

Apologies for the misunderstanding.  The offer to fix, or, perhaps
in lieu of this thread, enhance /usr/sbin/config still stands if
you'd like.  It might be fun to take the suck out of
/usr/sbin/config if people think it's warrented.  :-)

-- 
Adam - (http://people.migus.org/~amigus/)
Migus Dot Org - (http://www.migus.org/)



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