[PATCH] Fix variable assignments in function invocations

[PATCH] Fix variable assignments in function invocations

[Harald van Dijk]

[-- 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;

Re: [PATCH] Fix variable assignments in function invocations

[Eric Blake]

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

On 01/09/2015 10:17 AM, Harald van Dijk wrote:
> 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

This behavior is tricky.  Here's the latest POSIX wording:
http://austingroupbugs.net/view.php?id=654#c1559

      * If the command name is a function that is not a standard
        utility implemented as a function, variable assignments shall
        affect the current execution environment during the execution
        of the function. It is unspecified:

          - Whether or not the variable assignments persist
            after the completion of the function

          - Whether or not the variables gain the export
            attribute during the execution of the function

          - Whether or not export attributes gained as a result of the
            variable assignments persist after the completion of
            the function (if variable assignments persist after the
            completion of the function)

So the existing dash behavior is compliant, even if different from bash.

> 
> 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).

This is the text that was rendered obsolete by the above POSIX bug 654.

> 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?

I have no opinion on whether to take the patch in order to behave more
like bash, or whether to tell script-writers to fix their script to
avoid unspecified behavior because dash is already compliant in
providing a different behavior than bash.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [PATCH] Fix variable assignments in function invocations

[Harald van Dijk]

On 09/01/2015 18:39, Eric Blake wrote:
> On 01/09/2015 10:17 AM, Harald van Dijk wrote:
>> 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
>
> This behavior is tricky.  Here's the latest POSIX wording:
> http://austingroupbugs.net/view.php?id=654#c1559

 > [...]

> So the existing dash behavior is compliant, even if different from bash.

Thank you for the reference! I wasn't aware that the wording has since 
changed.

> I have no opinion on whether to take the patch in order to behave more
> like bash, or whether to tell script-writers to fix their script to
> avoid unspecified behavior because dash is already compliant in
> providing a different behavior than bash.

Well, perhaps it was already compliant, perhaps it became compliant by 
changing the standard, but either way, it is compliant now. :)

If either behaviour is allowed, then I believe dash aims to avoid 
breaking changes. Existing scripts written for dash could conceivably 
rely on the current behaviour, so without any real benefit, my patch 
would be a bad idea. bash compatibility is usually not considered a big 
benefit for dash, POSIX compatibility is.

Because of that, I withdraw my patch.

Cheers,
Harald van Dijk

Re: [PATCH] Fix variable assignments in function invocations

[Chet Ramey]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 1/9/15 12:39 PM, Eric Blake wrote:

> This behavior is tricky.  Here's the latest POSIX wording:
> http://austingroupbugs.net/view.php?id=654#c1559

The currently-published standard (as far as I know, I7 TC1 has not been
published yet) requires the dash behavior.  Bash implements it in
Posix mode.  Both shells will remain conformant when TC1 comes out.

See
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_05

which includes

"When a function is executed, it shall have the syntax-error and variable-
assignment properties described for special built-in utilities in the
enumerated list at the beginning of Special Built-In Utilities."

- -- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (Darwin)

iEYEARECAAYFAlSwLaYACgkQu1hp8GTqdKsSTgCfaLS1d4s/Mtr56BRVmiO37rDj
MooAnRQZ50E86M/xLRsngYRe33fDCxFa
=jlKx
-----END PGP SIGNATURE-----