Re: syscmd broken on FreeBSD and AIX

[Eric Blake]

On Fri, Sep 23, 2022 at 02:38:03AM +0200, Bruno Haible wrote:
> Hi,
> 
> With the current m4 from git (branch-1.4), several tests fail on FreeBSD 13
> and AIX 7.1 and 7.2, indicating that the syscmd built-in is completely
> dysfunctional there:

Thanks for the report.  I'm still thinking about this one...

> The problem can be reproduced interactively:
> 
> $ src/m4
> syscmd(`echo')
> echo: --:  not found.
> m4: syscmd subprocess failed
> 
> What happens, is that this invokes "sh -c -- echo". Which does not work. See:
> 
> $ sh -c -- echo
> echo: --: not found
> $ sh -c -- pwd
> pwd: --: not found
> $ sh -c -- foobar
> foobar: --: not found

That's a bug in sh not being POSIX-compliant.  But no one said sh has to be.

Instead of using -- to force sh to realize the command being passed is
intended to be a script to run (even if it starts with - or +), it is
also possible to prepend a space.  That is, if '-mycommand' is a real
command to be run, then a POSIX sh should treat:

sh -c -- -mycommand
sh -c ' -mycommand'

identically; and on non-POSIX sh, we avoid the misbehavior of -- not working.

I also like your idea of only special-casing command strings that
actually have the problem of a leading - or + (as those are rare),
rather than all command strings.

> 
> On Solaris 10, /bin/sh has the same problem, but the configuration of m4 sets
> SYSCMD_SHELL = "/usr/xpg4/bin/sh", thus working around the problem in an 
> elegant
> way.
> 
> On FreeBSD, bash may or may not be installed as /usr/local/bin/bash (part of
> the ports collection).
> On AIX, bash may or may not be installed as /usr/bin/bash (part of the Bull
> Freeware ports).
> 
> This is a regression, caused by the commit from 2021-11-19 with title
> "syscmd: Allow commands with leading - or +".

Yes, that patch was indeed the source of the regression, and it
stemmed from this discussion in POSIX:

https://www.austingroupbugs.net/view.php?id=1440

> 
> Find attached a patch that limits the new behaviour to the commands that
> actually start with - or +. With this, the set of failed tests shrinks to
> 
> Failed checks were:
>   ../../checks/006.command_li:err ../../checks/196.esyscmd:out 
> ../../checks/196.esyscmd:err ../../checks/197.sysval:out 
> ../../checks/197.sysval:err ../../checks/198.sysval:out 
> ../../checks/198.sysval:err ../../checks/199.mkstemp:err
> 
> The added line
>   prog_args[slot + 1] = NULL;
> is currently technically a nop, but makes the code more future-proof
> (in case more variation is needed in the prog_args array in the future).

I'm thinking of trying the alternative of not injecting "--" after
all, but instead munging the given argument to supply a leading space.
But that we need a new version of your patch.  Do you want to try and
rework it along those lines, or should I make time for that?

> +    {
> +      if (cmd[0] == '-' || cmd[0] == '+')
> +        /* If cmd starts with '-' or '+', "sh -c $cmd" is not guaranteed to
> +           work.  In this case, use "sh -c -- $cmd".
> +           Note: This requires a POSIX compliant SYSCMD_SHELL.  It does not
> +           work with /bin/sh on FreeBSD 13 and AIX 7, and on these platforms
> +           'bash' is not guaranteed to be installed.  */
> +        slot = 3;
> +      else
> +        /* For most commands, the traditional "sh -c $cmd" works fine.
> +           Including on FreeBSD 13 and AIX 7.  */
> +        slot = 2;
> +    }
>    prog_args[slot] = cmd;
> +  prog_args[slot + 1] = NULL;
>    errno = 0;
>    status = execute (ARG (0), SYSCMD_SHELL, prog_args, NULL, false,
>                      false, false, false, true, false, &sig_status);
> -- 
> 2.34.1
> 


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org