Bug#619725: dash: permit special builtin names as function names

[Jonathan Nieder]

reopen 619725 0.5.6.1-1~exp1
retitle 619725 dash: local should not be a special builtin
tags 619725 - patch
quit

Hi,

Jilles Tjoelker wrote:

 [false || local() { ...; } is a syntax error]

 Although I don't think special builtins shall be valid function names, I
 think there is still a bug in dash here: 'local' should not be a special
 builtin.

Thanks.

I'm torn about what to do in the long term.  Should the list of
special builtins stay the same forever?  Why should it be harder to
add a new special builtin than a new keyword?

But in the short term, the right thing to do is clear.  POSIX does not
give any latitude for inventing new special builtins, and there is
good reason for that, as you mentioned.



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#619725: dash: permit special builtin names as function names

[Jilles Tjoelker]

 [false || local() { ...; } is a syntax error]

Although I don't think special builtins shall be valid function names, I
think there is still a bug in dash here: 'local' should not be a special
builtin.

When 'local' was made a special builtin, I remarked on the dash mailing
list that that change may not be a good idea:
http://www.mail-archive.com/dash@vger.kernel.org/msg00268.html

I think this is a concrete reason why I was right. 'local' should be a
regular builtin and should be made to work in another way, such as by:
* adding a special case to evalcommand() for 'local' (if (spclbltin  0
  || argc == 0)), or
* modifying localcmd() so only local variables are only added to
  localvars lists belonging to functions.

Although I have not tried, the former looks easiest.

-- 
Jilles Tjoelker



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#619725: dash: permit special builtin names as function names

[Jonathan Nieder]

Package: dash
Version: 0.5.6.1-1~exp2
Severity: important
Justification: autogen ftbfs with dash as /bin/sh
Tags: upstream patch

$ texi2dvi
/usr/bin/texi2dvi: 144: /usr/bin/texi2dvi: Syntax error: Bad function name
$ sed -ne 144p /usr/bin/texi2dvi
) || local () {
$ dash -c 'true || local () { echo hi; }; echo continuing'; echo $?
dash: 1: Syntax error: Bad function name
2

texi2dvi tries to conditionally define a function named local.  What
that condition is is not important for now; the issue is that regardless
of the condition, dash errors out, considering such function names a
syntax error.  This condition || local () { :; } idiom is also used
by the autogen test suite.

POSIX says, regarding parsing of potential function names:

When the TOKEN is exactly a reserved word, the token
identifier for that reserved word shall result. Otherwise,
when the TOKEN meets the requirements for a name, the token
identifier NAME shall result. Otherwise, rule 7 applies.

Names of special builtins meet the requirements for names and are not
reserved words.  So this is not, after all, a syntax error.

A case could be made that it _ought_ to be a syntax error.  A function
with such a name could never be called, since according to 2.9.1.1.1
Command Search and Execution, special builtins take precedence over
functions in the command search.  And there has been talk of making
assignment builtins like local into reserved words, too.  But it
seems best for now to implement the behavior that is actually
standardized, especially given that some people are relying on it.

Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Rough patch follows (untested).

 src/parser.c |7 +--
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/src/parser.c b/src/parser.c
index 528d005..244d9ac 100644
--- a/src/parser.c
+++ b/src/parser.c
@@ -545,12 +545,7 @@ simplecmd(void) {
if (readtoken() != TRP)
synexpect(TRP);
name = n-narg.text;
-   if (
-   !goodname(name) || (
-   (bcmd = find_builtin(name)) 
-   bcmd-flags  BUILTIN_SPECIAL
-   )
-   )
+   if (!goodname(name))
synerror(Bad function name);
n-type = NDEFUN;
checkkwd = CHKNL | CHKKWD | CHKALIAS;
-- 
1.7.4.1




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org