Re: [code] [textadept] Curses `os.spawn` Shell String Parsing

From: Chris Emerson <c-ta.att.mail.nosreme.org>
Date: Thu, 13 Dec 2018 10:04:50 +0000

Hi,

As another curses user, that seems reasonable (though it would of course
potentially break existing setups - but Textadept seems good at embracing
minor breaks for longer-term improvement. :-) )

On the topic of os.spawn, I've recently got to the bottom of a problem I've
had with it: some programs behave differently when they have a pipe as stdin.
I keep having ripgrep appearing to hang - it sees a pipe as stdin and assumes
it should read from that instead of searching from the current directory.

So it would be useful to have an option to set stdin of the spawned process
to /dev/null instead of a pipe that won't be used. (I can work around it by
indirecting through a one-line shell script, of course).

Chris

On Wed, Dec 12, 2018 at 08:39:12PM -0800, Chad Voegele wrote:
> Hi Mitchell,
>
> I use `sed` with `filter_through` for search and replace and have been
> having trouble when trying to replace single quotes with double
> quotes, i.e. `filter_through('sed "s/\'/\\"/g"')`. I narrowed the
> issue to an implementation difference where the curses version uses
> custom logic to parse the shell string. Lines 807-817 in the patched
> `loslib.c` seem to have trouble with the escaped double quote.
>
> Since I'm guessing you would not like to have a `glib` dependency in
> the curses version, one option to fix this could be to use the POSIX
> function `wordexp` provided by `libc`. Please see the patch below for
> details. Would you be willing to consider adopting this patch or
> something similar?
>
> Thanks!
> Chad
>
> --- /tmp/a/loslib.c 2018-12-12 19:36:01.314978736 -0800
> +++ /tmp/b/loslib.c 2018-12-12 19:57:47.704907970 -0800
> @@ -441,6 +441,7 @@
> #if (!GTK || __APPLE__)
> #include <errno.h>
> #include <sys/select.h>
> +#include <wordexp.h>
> #endif
> #include <sys/wait.h>
> #include <signal.h>
> @@ -788,6 +789,19 @@
> #endif
> #endif
>
> +static const char* wordexp_error_to_message(int status) {
> + switch (status) {
> + case WRDE_SYNTAX:
> + return "Syntax error";
> + case WRDE_NOSPACE:
> + return "Out of memory";
> + case WRDE_BADCHAR:
> + return "Invalid character";
> + default:
> + return "Unknown error";
> + }
> +}
> +
> /** spawn() Lua function. */
> static int os_spawn(lua_State *L) {
> int narg = 1;
> @@ -802,31 +816,13 @@
> luaL_argerror(L, 1, lua_tostring(L, -1));
> }
> #else
> - lua_newtable(L);
> - const char *param = luaL_checkstring(L, narg++), *c = param;
> - while (*c) {
> - while (*c == ' ') c++;
> - param = c;
> - if (*c == '"') {
> - param = ++c;
> - while (*c && *c != '"') c++;
> - } else while (*c && *c != ' ') c++;
> - lua_pushlstring(L, param, c - param);
> - lua_rawseti(L, -2, lua_rawlen(L, -2) + 1);
> - if (*c == '"') c++;
> - }
> - int argc = lua_rawlen(L, -1);
> - char **argv = malloc((argc + 1) * sizeof(char *));
> - for (int i = 0; i < argc; i++) {
> - lua_rawgeti(L, -1, i + 1);
> - size_t len = lua_rawlen(L, -1);
> - char *param = malloc(len + 1);
> - strcpy(param, lua_tostring(L, -1)), param[len] = '\0';
> - argv[i] = param;
> - lua_pop(L, 1); // param
> + const char *param = luaL_checkstring(L, narg++);
> + wordexp_t wordexp_argv;
> + int status = wordexp(param, &wordexp_argv, 0);
> + if(status) {
> + lua_pushfstring(L, "invalid argv: %s", wordexp_error_to_message(status));
> + luaL_argerror(L, 1, lua_tostring(L, -1));
> }
> - argv[argc] = NULL;
> - lua_pop(L, 1); // argv
> #endif
> // Determine cwd from optional second string param.
> const char *cwd = lua_isstring(L, narg) ? lua_tostring(L, narg++) : NULL;
> @@ -954,12 +950,12 @@
> extern char **environ;
> #if __linux__
> if (!envp) envp = environ;
> - execvpe(argv[0], argv, envp); // does not return on success
> + execvpe(wordexp_argv.we_wordv[0], wordexp_argv.we_wordv, envp);
> // does not return on success
> #else
> if (envp) environ = envp;
> - execvp(argv[0], argv); // does not return on success
> + execvp(wordexp_argv.we_wordv[0], wordexp_argv.we_wordv); //
> does not return on success
> #endif
> - fprintf(stderr, "Failed to execute child process \"%s\" (%s)", argv[0],
> + fprintf(stderr, "Failed to execute child process \"%s\" (%s)",
> wordexp_argv.we_wordv[0],
> strerror(errno));
> exit(EXIT_FAILURE);
> }
> @@ -970,8 +966,7 @@
> lua_pushnil(L);
> lua_pushfstring(L, "%s: %s", lua_tostring(L, 1), strerror(errno));
> }
> - for (int i = 0; i < argc; i++) free(argv[i]);
> - free(argv);
> + wordfree(&wordexp_argv);
> if (envp) {
> for (int i = 0; i < envn; i++) free(envp[i]);
> free(envp);
> --
> You are subscribed to code.att.foicica.com.
> To change subscription settings, send an e-mail to code+help.att.foicica.com.
> To unsubscribe, send an e-mail to code+unsubscribe.att.foicica.com.
>

-- 
You are subscribed to code.att.foicica.com.
To change subscription settings, send an e-mail to code+help.att.foicica.com.
To unsubscribe, send an e-mail to code+unsubscribe.att.foicica.com.
Received on Thu 13 Dec 2018 - 05:04:50 EST

This archive was generated by hypermail 2.2.0 : Thu 13 Dec 2018 - 06:38:21 EST