dash.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Harald van Dijk <harald@gigawatt.nl>
To: dash <dash@vger.kernel.org>
Subject: [PATCH] Fix variable assignments in function invocations
Date: Fri, 09 Jan 2015 18:17:42 +0100	[thread overview]
Message-ID: <54B00D36.7070707@gigawatt.nl> (raw)

[-- Attachment #1: Type: text/plain, Size: 1637 bytes --]

Hello all,

A long-standing problem with dash has been how it deals with variable 
assignments in function invocations, and several packages are affected 
by it, two I've come across recently being autogen and pkg-config (only 
their test suites, luckily).

A short test script:

   f() {
     echo inside f, VAR is $VAR
     sh -c 'echo inside sh called from f, VAR is $VAR'
   }

   VAR=value f
   echo after returning from f, VAR is $VAR

Assuming VAR was not already set, this should print (and does with bash):

   inside f, VAR is value
   inside sh called from f, VAR is value
   after returning from f, VAR is

With dash, this actually prints:

   inside f, VAR is value
   inside sh called from f, VAR is
   after returning from f, VAR is value

The first problem with that is that VAR does not get exported, the 
second is that VAR's assigned value is kept after the function has returned.

Quoting SUSv4 Shell Command Language 2.9.1 Simple Commands:

   If no command name results, variable assignments shall affect the
   current execution environment. Otherwise, the variable assignments
   shall be exported for the execution environment of the command and
   shall not affect the current execution environment (except for
   special built-ins).

In `VAR=value f`, f is found as the command name. No exception is made 
for function invocations, so I believe this disallows dash's current 
behaviour, and requires it to print the same thing bash does.

Fixing this seems trivial, see the attachment, and the test suites of 
both autogen and pkg-config pass with this change. Does this look correct?

Cheers,
Harald van Dijk

[-- Attachment #2: funcvars.patch --]
[-- Type: text/plain, Size: 236 bytes --]

--- a/src/eval.c
+++ b/src/eval.c
@@ -875,7 +875,7 @@ raise:
 		break;
 
 	case CMDFUNCTION:
-		poplocalvars(1);
+		listsetvar(varlist.list, VEXPORT|VSTACK);
 		if (evalfun(cmdentry.u.func, argc, argv, flags))
 			goto raise;
 		break;

             reply	other threads:[~2015-01-09 17:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-09 17:17 Harald van Dijk [this message]
2015-01-09 17:39 ` Eric Blake
2015-01-09 17:55   ` Harald van Dijk
2015-01-09 19:36   ` Chet Ramey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54B00D36.7070707@gigawatt.nl \
    --to=harald@gigawatt.nl \
    --cc=dash@vger.kernel.org \
    --subject='Re: [PATCH] Fix variable assignments in function invocations' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).