Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL

[Omar Polo]

On 2023/03/27 18:58:07 -0600, Todd C. Miller <mill...@openbsd.org> wrote:
> On Mon, 27 Mar 2023 20:06:30 +0200, Omar Polo wrote:
> 
> > Is _PATH_BSHELL portable though?  I can see a few stuff that uses it
> > (vi among others) but I'm not sure.
> 
> The paths.h header is a BSD invention, though it may be present on
> some other systems.  I don't think that's a reason not to use it
> though.
> 
> > Also, if you look at the callers of shellcmdoutput() they all prepare
> > the argv array as {"sh", "-c", "command provided", NULL} so I'm
> > wondering if we should just ignore $SHELL and always use /bin/sh, or
> > change that "sh" accordingly to $SHELL.
> 
> It might be best to use the basename of the actual shell for argv[0].
> Our ksh for instance has slightly different behavior when invoked
> as sh.

like this? :)

(need an up-to-date tree since it builds on the previous, committed,
diff.)

I've tested with ksh and csh.  I guess it's fine to assume the user'
shell has a -c flag.  (vi does the same.)


Thanks!

Index: region.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/region.c,v
retrieving revision 1.43
diff -u -p -r1.43 region.c
--- region.c    28 Mar 2023 08:01:40 -0000      1.43
+++ region.c    28 Mar 2023 08:17:30 -0000
@@ -34,7 +34,7 @@ static        int     iomux(int, char * const, int,
 static int     preadin(int, struct buffer *);
 static void    pwriteout(int, char **, int *);
 static int     setsize(struct region *, RSIZE);
-static int     shellcmdoutput(char * const[], char * const, int);
+static int     shellcmdoutput(char * const, char * const, int);
 
 /*
  * Kill the region.  Ask "getregion" to figure out the bounds of the region.
@@ -415,7 +415,6 @@ piperegion(int f, int n)
        struct region region;
        int len;
        char *cmd, cmdbuf[NFILEN], *text;
-       char *argv[] = {"sh", "-c", (char *) NULL, (char *) NULL};
 
        /* C-u M-| is not supported yet */
        if (n > 1)
@@ -431,8 +430,6 @@ piperegion(int f, int n)
            EFNEW | EFCR)) == NULL || (cmd[0] == '\0'))
                return (ABORT);
 
-       argv[2] = cmd;
-
        if (getregion(&region) != TRUE)
                return (FALSE);
 
@@ -446,7 +443,7 @@ piperegion(int f, int n)
 
        region_get_data(&region, text, len);
 
-       return shellcmdoutput(argv, text, len);
+       return shellcmdoutput(cmd, text, len);
 }
 
 /*
@@ -456,7 +453,6 @@ int
 shellcommand(int f, int n)
 {
        char *cmd, cmdbuf[NFILEN];
-       char *argv[] = {"sh", "-c", (char *) NULL, (char *) NULL};
 
        if (n > 1)
                return (ABORT);
@@ -465,16 +461,15 @@ shellcommand(int f, int n)
            EFNEW | EFCR)) == NULL || (cmd[0] == '\0'))
                return (ABORT);
 
-       argv[2] = cmd;
-
-       return shellcmdoutput(argv, NULL, 0);
+       return shellcmdoutput(cmd, NULL, 0);
 }
 
 int
-shellcmdoutput(char* const argv[], char* const text, int len)
+shellcmdoutput(char* const cmd, char* const text, int len)
 {
        struct buffer *bp;
-       char    *shellp;
+       char    *argv[] = {NULL, "-c", cmd, NULL};
+       char    *shellp, *shell;
        int      ret;
 
        bp = bfind("*Shell Command Output*", TRUE);
@@ -486,6 +481,10 @@ shellcmdoutput(char* const argv[], char*
 
        if ((shellp = getenv("SHELL")) == NULL)
                shellp = _PATH_BSHELL;
+
+       if ((shell = strrchr(shellp, '/')) == NULL)
+               shell = shellp;
+       argv[0] = shell;
 
        ret = pipeio(shellp, argv, text, len, bp);