[PATCH 1/2] [SHELL] Allow building without LINEO support.

[PATCH 1/2] [SHELL] Allow building without LINEO support.

[David Miller]

Simply specify --disable-lineno to configure.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 configure.ac |    5 +++++
 src/var.c    |    4 ++++
 src/var.h    |    6 ++++++
 3 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7eae954..96b6310 100644
--- a/configure.ac
+++ b/configure.ac
@@ -131,5 +131,10 @@ if test "$use_libedit" != "yes"; then
 else
 	export LIBS="$LIBS -ledit"
 fi
+AC_ARG_ENABLE(lineno, AS_HELP_STRING(--disable-lineno, \
+				     [Disable LINENO support]))
+if test "$enable_lineno" != "no"; then
+	AC_DEFINE([WITH_LINENO], 1, [Define if you build with -DWITH_LINENO])
+fi
 AC_CONFIG_FILES([Makefile src/Makefile])
 AC_OUTPUT
diff --git a/src/var.c b/src/var.c
index ecc8c90..027beff 100644
--- a/src/var.c
+++ b/src/var.c
@@ -101,7 +101,9 @@ struct var varinit[] = {
 	{ 0,	VSTRFIXED|VTEXTFIXED,		"PS2=> ",	0 },
 	{ 0,	VSTRFIXED|VTEXTFIXED,		"PS4=+ ",	0 },
 	{ 0,	VSTRFIXED|VTEXTFIXED,		"OPTIND=1",	getoptsreset },
+#ifdef WITH_LINENO
 	{ 0,	VSTRFIXED|VTEXTFIXED,		linenovar,	0 },
+#endif
 #ifndef SMALL
 	{ 0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"TERM\0",	0 },
 	{ 0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"HISTSIZE\0",	sethistsize },
@@ -335,9 +337,11 @@ lookupvar(const char *name)
 	struct var *v;
 
 	if ((v = *findvar(hashvar(name), name)) && !(v->flags & VUNSET)) {
+#ifdef WITH_LINENO
 		if (v == &vlineno && v->text == linenovar) {
 			fmtstr(linenovar+7, sizeof(linenovar)-7, "%d", lineno);
 		}
+#endif
 		return strchrnul(v->text, '=') + 1;
 	}
 	return NULL;
diff --git a/src/var.h b/src/var.h
index 54f7b2d..79ee71a 100644
--- a/src/var.h
+++ b/src/var.h
@@ -88,9 +88,15 @@ extern struct var varinit[];
 #define vps2 (&vps1)[1]
 #define vps4 (&vps2)[1]
 #define voptind (&vps4)[1]
+#ifdef WITH_LINENO
 #define vlineno (&voptind)[1]
+#endif
 #ifndef SMALL
+#ifdef WITH_LINENO
 #define vterm (&vlineno)[1]
+#else
+#define vterm (&voptind)[1]
+#endif
 #define vhistsize (&vterm)[1]
 #endif
 
-- 
1.7.6.401.g6a319

Re: [PATCH 1/2] [SHELL] Allow building without LINEO support.

[Jonathan Nieder]

David Miller wrote:

> [Subject: [SHELL] Allow building without LINEO support.]

Thanks!  Debian has been using something like this (but unconditional)
to convince autoconf not to use dash as CONFIG_SHELL, to work around
bugs in various configure scripts[1].  I imagine other users might
want the same thing, so a patch like this seems like a good idea.

I think LINEO should be LINENO in the subject line.

[1] http://bugs.debian.org/582952

[...]
> --- a/configure.ac
> +++ b/configure.ac
> @@ -131,5 +131,10 @@ if test "$use_libedit" != "yes"; then
>  else
>  	export LIBS="$LIBS -ledit"
>  fi
> +AC_ARG_ENABLE(lineno, AS_HELP_STRING(--disable-lineno, \
> +				     [Disable LINENO support]))
> +if test "$enable_lineno" != "no"; then
> +	AC_DEFINE([WITH_LINENO], 1, [Define if you build with -DWITH_LINENO])
> +fi

This produces the following in config.h:

	/* Define if you build with -DWITH_LINENO */
	#define WITH_LINENO 1

Something like "Define if you want support for the LINENO variable"
might be a little more satisfying.

> --- a/src/var.c
> +++ b/src/var.c
> @@ -101,7 +101,9 @@ struct var varinit[] = {
>  	{ 0,	VSTRFIXED|VTEXTFIXED,		"PS2=> ",	0 },
>  	{ 0,	VSTRFIXED|VTEXTFIXED,		"PS4=+ ",	0 },
>  	{ 0,	VSTRFIXED|VTEXTFIXED,		"OPTIND=1",	getoptsreset },
> +#ifdef WITH_LINENO
>  	{ 0,	VSTRFIXED|VTEXTFIXED,		linenovar,	0 },
> +#endif
>  #ifndef SMALL
>  	{ 0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"TERM\0",	0 },
[etc]

Makes sense.  The following on top trims away some code in the
!WITH_LINENO case, though it's kind of ugly.  Maybe something like

	#ifndef WITH_LINENO
	static void update_lineno(void) { }
	#else
	static void update_lineno(void)
	{
		lineno = errlinno;
		if (funcline)
			lineno -= funcline - 1;
	}
	#endif

would make it more palatable.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 src/eval.c |   25 ++++++++++++++++++++-----
 src/var.c  |    2 ++
 src/var.h  |    4 ++++
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index c7358a62..1184ff10 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -213,9 +213,12 @@ evaltree(union node *n, int flags)
 		status = !exitstatus;
 		goto setstatus;
 	case NREDIR:
-		errlinno = lineno = n->nredir.linno;
+		errlinno = n->nredir.linno;
+#ifdef WITH_LINENO
+		lineno = errlinno;
 		if (funcline)
 			lineno -= funcline - 1;
+#endif
 		expredir(n->nredir.redirect);
 		pushredir(n->nredir.redirect);
 		status = redirectsafe(n->nredir.redirect, REDIR_PUSH);
@@ -374,9 +377,12 @@ evalfor(union node *n, int flags)
 	struct strlist *sp;
 	struct stackmark smark;
 
-	errlinno = lineno = n->nfor.linno;
+	errlinno = n->nfor.linno;
+#ifdef WITH_LINENO
+	lineno = errlinno;
 	if (funcline)
 		lineno -= funcline - 1;
+#endif
 
 	setstackmark(&smark);
 	arglist.lastp = &arglist.list;
@@ -419,9 +425,12 @@ evalcase(union node *n, int flags)
 	struct arglist arglist;
 	struct stackmark smark;
 
-	errlinno = lineno = n->ncase.linno;
+	errlinno = n->ncase.linno;
+#ifdef WITH_LINNO
+	lineno = errlinno;
 	if (funcline)
 		lineno -= funcline - 1;
+#endif
 
 	setstackmark(&smark);
 	arglist.lastp = &arglist.list;
@@ -454,9 +463,12 @@ evalsubshell(union node *n, int flags)
 	int backgnd = (n->type == NBACKGND);
 	int status;
 
-	errlinno = lineno = n->nredir.linno;
+	errlinno = n->nredir.linno;
+#ifdef WITH_LINENO
+	lineno = errlinno;
 	if (funcline)
 		lineno -= funcline - 1;
+#endif
 
 	expredir(n->nredir.redirect);
 	if (!backgnd && flags & EV_EXIT && !have_traps())
@@ -689,9 +701,12 @@ evalcommand(union node *cmd, int flags)
 	int status;
 	char **nargv;
 
-	errlinno = lineno = cmd->ncmd.linno;
+	errlinno = cmd->ncmd.linno;
+#ifdef WITH_LINENO
+	lineno = errlinno;
 	if (funcline)
 		lineno -= funcline - 1;
+#endif
 
 	/* First expand the arguments. */
 	TRACE(("evalcommand(0x%lx, %d) called\n", (long)cmd, flags));
diff --git a/src/var.c b/src/var.c
index 027beff1..8e125701 100644
--- a/src/var.c
+++ b/src/var.c
@@ -81,8 +81,10 @@ const char defifsvar[] = "IFS= \t\n";
 const char defifs[] = " \t\n";
 #endif
 
+#ifdef WITH_LINENO
 int lineno;
 char linenovar[sizeof("LINENO=")+sizeof(int)*CHAR_BIT/3+1] = "LINENO=";
+#endif
 
 /* Some macros in var.h depend on the order, add new variables to the end. */
 struct var varinit[] = {
diff --git a/src/var.h b/src/var.h
index 79ee71a6..533ee0de 100644
--- a/src/var.h
+++ b/src/var.h
@@ -109,8 +109,10 @@ extern const char defifs[];
 extern const char defpathvar[];
 #define defpath (defpathvar + 5)
 
+#ifdef WITH_LINENO
 extern int lineno;
 extern char linenovar[];
+#endif
 
 /*
  * The following macros access the values of the above variables.
@@ -127,7 +129,9 @@ extern char linenovar[];
 #define ps2val()	(vps2.text + 4)
 #define ps4val()	(vps4.text + 4)
 #define optindval()	(voptind.text + 7)
+#ifdef WITH_LINENO
 #define linenoval()	(vlineno.text + 7)
+#endif
 #ifndef SMALL
 #define histsizeval()	(vhistsize.text + 9)
 #define termval()	(vterm.text + 5)
-- 
1.7.6

Re: [PATCH 1/2] [SHELL] Allow building without LINEO support.

[Herbert Xu]

On Tue, Aug 16, 2011 at 05:45:02PM -0700, David Miller wrote:
> 
> Simply specify --disable-lineno to configure.
> 
> Signed-off-by: David S. Miller <davem@davemloft.net>

Both patches applied.  Thanks a lot!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 1/2] [SHELL] Allow building without LINEO support.

[Herbert Xu]

On Tue, Aug 16, 2011 at 08:12:54PM -0500, Jonathan Nieder wrote:
> David Miller wrote:
> 
> > [Subject: [SHELL] Allow building without LINEO support.]
> 
> Thanks!  Debian has been using something like this (but unconditional)
> to convince autoconf not to use dash as CONFIG_SHELL, to work around
> bugs in various configure scripts[1].  I imagine other users might
> want the same thing, so a patch like this seems like a good idea.
> 
> I think LINEO should be LINENO in the subject line.
> 
> [1] http://bugs.debian.org/582952

So this isn't the reason why Debian has been stuck on 0.5.5.1?
If so what is the reason please?

> Makes sense.  The following on top trims away some code in the
> !WITH_LINENO case, though it's kind of ugly.  Maybe something like
> 
> 	#ifndef WITH_LINENO
> 	static void update_lineno(void) { }
> 	#else
> 	static void update_lineno(void)
> 	{
> 		lineno = errlinno;
> 		if (funcline)
> 			lineno -= funcline - 1;
> 	}
> 	#endif
> 
> would make it more palatable.

Yes please do it like that.

Thanks!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 1/2] [SHELL] Allow building without LINEO support.

[Jonathan Nieder]

Herbert Xu wrote:

> So this isn't the reason why Debian has been stuck on 0.5.5.1?
> If so what is the reason please?

Probably just Gerrit being busy and me getting distracted by how
to handle the /bin/sh symlink.

I'll propose a package for sid that just moves to the latest upstream
version with no packaging changes later today.

>> Makes sense.  The following on top trims away some code in the
>> !WITH_LINENO case, though it's kind of ugly.  Maybe something like
>>
>> 	#ifndef WITH_LINENO
>> 	static void update_lineno(void) { }
>> 	#else
>> 	static void update_lineno(void)
[...]
>> would make it more palatable.
>
> Yes please do it like that.

Will do this as well.

Thanks,
Jonathan

Re: [PATCH 1/2] [SHELL] Allow building without LINEO support.

[Harald van Dijk]

On Tue, 2011-08-16 at 20:12 -0500, Jonathan Nieder wrote:
> David Miller wrote:
> 
> > [Subject: [SHELL] Allow building without LINEO support.]
> 
> Thanks!  Debian has been using something like this (but unconditional)
> to convince autoconf not to use dash as CONFIG_SHELL, to work around
> bugs in various configure scripts[1].  I imagine other users might
> want the same thing, so a patch like this seems like a good idea.

If you don't mind me asking, if you want configure scripts to run from
bash, why not simply run configure scripts from bash, instead of running
them from sh and trusting that sh will call bash if it is really some
other shell? Don't get me wrong, there are other reasons why making
LINENO support optional is a good idea (smaller executable at the low
cost of a rarely used feature), so don't take this as an argument
against this patch, I'm just curious about this one point.

Re: [PATCH 1/2] [SHELL] Allow building without LINEO support.

[Eric Blake]

On 08/17/2011 12:04 AM, Harald van Dijk wrote:
> On Tue, 2011-08-16 at 20:12 -0500, Jonathan Nieder wrote:
>> David Miller wrote:
>>
>>> [Subject: [SHELL] Allow building without LINEO support.]
>>
>> Thanks!  Debian has been using something like this (but unconditional)
>> to convince autoconf not to use dash as CONFIG_SHELL, to work around
>> bugs in various configure scripts[1].  I imagine other users might
>> want the same thing, so a patch like this seems like a good idea.
>
> If you don't mind me asking, if you want configure scripts to run from
> bash, why not simply run configure scripts from bash, instead of running
> them from sh and trusting that sh will call bash if it is really some
> other shell?

And remember, most configure scripts already support that:

CONFIG_SHELL=path/to/bash path/to/bash ./configure

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org