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