Skip to contentSkip to conversation

parser: Fix handling of empty aliases

PATCH In review Herbert Xu 3 participants 3 messages Apr 27, 2020 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Herbert Xu

author
#1 lore raw

PATCH v2 1/1 parser: Fix handling of empty aliases

Quoted 19 lines · Michael Greenberg

Dash was incorrectly handling empty aliases. When attempting to use an empty alias with nothing else, I'm (incorrectly) prompted for more input:

```
$ alias empty=''
$ empty
>
```

Other shells (e.g., bash, yash) correctly handle the lone, empty alias as an empty command:

```
$ alias empty=''
$ empty
$
```
Quoted 4 lines

This patch fixes the parser to handle the case where an alias is empty, i.e., produces no token.

Thanks for the patch! I think your patch doesn't fix all the cases though as it's also possible to encounter the problem if the alias consists entirely of space characters. Here is my attempt at the problem.

---8<---
Dash was incorrectly handling empty aliases. When attempting to use an empty alias with nothing else, I'm (incorrectly) prompted for more input:

```
$ alias empty=''
$ empty
>
```

Other shells (e.g., bash, yash) correctly handle the lone, empty alias as an empty command:

```
$ alias empty=''
$ empty
$
```

The problem here is that we incorrectly enter the loop eating TNLs in readtoken(). This patch fixes it by setting checkkwd correctly.

src/parser.c +10 -19
index b318b08..5c9e9a0 100644
@@ -115,7 +115,6 @@ STATIC union node *simplecmd(void);
 STATIC union node *makename(void);
 STATIC void parsefname(void);
 STATIC void parseheredoc(void);
-STATIC int peektoken(void);
 STATIC int readtoken(void);
 STATIC int xxreadtoken(void);
 STATIC int pgetc_eatbnl();
@@ -161,21 +160,23 @@ parsecmd(int interact)
 STATIC union node *
 list(int nlflag)
 {
+	int chknl = nlflag & 1 ? 0 : CHKNL;
 	union node *n1, *n2, *n3;
 	int tok;
 
 	n1 = NULL;
 	for (;;) {
-		switch (readtoken()) {
+		checkkwd = chknl | CHKKWD | CHKALIAS;
+		tok = readtoken();
+		switch (tok) {
 		case TNL:
-			if (!(nlflag & 1))
-				break;
 			parseheredoc();
 			return n1;
 
 		case TEOF:
-			if (!n1 && (nlflag & 1))
+			if (!n1 && !chknl)
 				n1 = NEOF;
+out_eof:
 			parseheredoc();
 			tokpushback++;
 			lasttoken = TEOF;
@@ -183,8 +184,7 @@ list(int nlflag)
 		}
 
 		tokpushback++;
-		checkkwd = CHKNL | CHKKWD | CHKALIAS;
-		if (nlflag == 2 && tokendlist[peektoken()])
+		if (nlflag == 2 && tokendlist[tok])
 			return n1;
 		nlflag |= 2;
 
@@ -214,15 +214,16 @@ list(int nlflag)
 			n1 = n3;
 		}
 		switch (tok) {
-		case TNL:
 		case TEOF:
+			goto out_eof;
+		case TNL:
 			tokpushback++;
 			/* fall through */
 		case TBACKGND:
 		case TSEMI:
 			break;
 		default:
-			if ((nlflag & 1))
+			if (!chknl)
 				synexpect(-1);
 			tokpushback++;
 			return n1;
@@ -685,16 +686,6 @@ parseheredoc(void)
 	}
 }
 
-STATIC int
-peektoken(void)
-{
-	int t;
-
-	t = readtoken();
-	tokpushback++;
-	return (t);
-}
-
 STATIC int
 readtoken(void)
 {
Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#2 lore raw
Quoted 23 lines · Herbert Xu
Quoted 22 lines · Michael Greenberg

Dash was incorrectly handling empty aliases. When attempting to use an empty alias with nothing else, I'm (incorrectly) prompted for more input:

```
$ alias empty=''
$ empty
>
```

Other shells (e.g., bash, yash) correctly handle the lone, empty alias as an empty command:

```
$ alias empty=''
$ empty
$
```

This patch fixes the parser to handle the case where an alias is empty, i.e., produces no token.

Quoted 6 lines
Quoted 1 line

Thanks for the patch! I think your patch doesn't fix all the cases though as it's also possible to encounter the problem if the alias consists entirely of space characters. Here is my attempt at the problem.

Yiiikes, that's subtle. Good catch, thanks! :)

I don't think the current documentation is enough for me to have implemented your fix even if I had thought of the issue with spaces. If someone who understands the parser well wants to spend some time documenting it better, I'm sure it would come in handy for other folks.

Cheers,
Michael

Quoted 119 lines

---8<---
Dash was incorrectly handling empty aliases. When attempting to use an empty alias with nothing else, I'm (incorrectly) prompted for more input:

```
$ alias empty=''
$ empty
>
```

Other shells (e.g., bash, yash) correctly handle the lone, empty alias as an empty command:

```
$ alias empty=''
$ empty
$
```

The problem here is that we incorrectly enter the loop eating TNLs in readtoken(). This patch fixes it by setting checkkwd correctly.

src/parser.c +10 -20
index b318b08..5c9e9a0 100644
@@ -115,7 +115,6 @@ STATIC union node *simplecmd(void);
 STATIC union node *makename(void);
 STATIC void parsefname(void);
 STATIC void parseheredoc(void);
-STATIC int peektoken(void);
 STATIC int readtoken(void);
 STATIC int xxreadtoken(void);
 STATIC int pgetc_eatbnl();
@@ -161,21 +160,23 @@ parsecmd(int interact)
 STATIC union node *
 list(int nlflag)
 {
+       int chknl = nlflag & 1 ? 0 : CHKNL;
        union node *n1, *n2, *n3;
        int tok;
 
        n1 = NULL;
        for (;;) {
-               switch (readtoken()) {
+               checkkwd = chknl | CHKKWD | CHKALIAS;
+               tok = readtoken();
+               switch (tok) {
                case TNL:
-                       if (!(nlflag & 1))
-                               break;
                        parseheredoc();
                        return n1;
 
                case TEOF:
-                       if (!n1 && (nlflag & 1))
+                       if (!n1 && !chknl)
                                n1 = NEOF;
+out_eof:
                        parseheredoc();
                        tokpushback++;
                        lasttoken = TEOF;
@@ -183,8 +184,7 @@ list(int nlflag)
                }
 
                tokpushback++;
-               checkkwd = CHKNL | CHKKWD | CHKALIAS;
-               if (nlflag == 2 && tokendlist[peektoken()])
+               if (nlflag == 2 && tokendlist[tok])
                        return n1;
                nlflag |= 2;
 
@@ -214,15 +214,16 @@ list(int nlflag)
                        n1 = n3;
                }
                switch (tok) {
-               case TNL:
                case TEOF:
+                       goto out_eof;
+               case TNL:
                        tokpushback++;
                        /* fall through */
                case TBACKGND:
                case TSEMI:
                        break;
                default:
-                       if ((nlflag & 1))
+                       if (!chknl)
                                synexpect(-1);
                        tokpushback++;
                        return n1;
@@ -685,16 +686,6 @@ parseheredoc(void)
        }
 }
 
-STATIC int
-peektoken(void)
-{
-       int t;
-
-       t = readtoken();
-       tokpushback++;
-       return (t);
-}
-
 STATIC int
 readtoken(void)
 {
--

Email: Herbert Xu <[email protected]>
Home Page: https://nam01.safelinks.protection.outlook.com/?url=http:%2F%2Fgondor.apana.org.au%2F~herbert%2F&amp;data=02%7C01%7CMichael.Greenberg%40pomona.edu%7C5d04fbe7205242dfede408d7eabdd3b2%7C817f590439044ee8b3a5a65d4746ff70%7C0%7C0%7C637235973354290743&amp;sdata=yhd8GJ2xr4Pemyl3NWaN10vwYSWjI35bBWIBZlv0eis%3D&amp;reserved=0
PGP Key: https://nam01.safelinks.protection.outlook.com/?url=http:%2F%2Fgondor.apana.org.au%2F~herbert%2Fpubkey.txt&amp;data=02%7C01%7CMichael.Greenberg%40pomona.edu%7C5d04fbe7205242dfede408d7eabdd3b2%7C817f590439044ee8b3a5a65d4746ff70%7C0%7C0%7C637235973354290743&amp;sdata=MK3ePSn71XHOmhfYIHr8lnKBnVQ%2BwEvEdZb%2BcLxGH%2F4%3D&amp;reserved=0
________________________________

[EXTERNAL EMAIL] Exercise caution before clicking on links or opening attachments.

#3 lore raw

Re: [PATCH] alias: Fix handling of empty aliases

In reply to a message not in the archive

Op 09-05-19 om 16:46 schreef Michael Greenberg:

Quoted 1 line

Dash was incorrectly handling empty aliases.

For the record, the behaviour in Busybox ash, FreeBSD sh and NetBSD sh is the same.

- M.

Signature · 2 lines
modernish -- harness the shell
https://github.com/modernish/modernish