Re: [dev] [st][PATCH] better plumbing on linux: find child shell cwd without shell gymnastics

From: Markus Wichmann <nullplan_AT_gmx.net>
Date: Thu, 1 Nov 2018 13:43:18 +0100

On Thu, Nov 01, 2018 at 08:46:06AM +0100, John Soros wrote:
> config.def.h | 6 +++++
> x.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 82 insertions(+)
>
> diff --git a/config.def.h b/config.def.h
> index 87ebdbb..265a4ff 100644
> --- a/config.def.h
> +++ b/config.def.h
> _AT_@ -467,3 +467,9 @@ static char ascii_printable[] =
> " !\"#$%&'()*+,-./0123456789:;<=>?"
> "_AT_ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_"
> "`abcdefghijklmnopqrstuvwxyz{|}~";
> +
> +/*
> + * plumber_cmd is run on mouse button 3 click, with argument set to
> + * current selection and with cwd set to the cwd of the active shell
> + */
> +static char *plumber_cmd = "plumb";
> diff --git a/x.c b/x.c
> index 4345892..f319129 100644
> --- a/x.c
> +++ b/x.c
> _AT_@ -5,6 +5,9 @@
> #include <locale.h>
> #include <signal.h>
> #include <sys/select.h>
> +#include <sys/stat.h>
> +#include <sys/wait.h>
> +#include <dirent.h>
> #include <time.h>
> #include <unistd.h>
> #include <libgen.h>
> _AT_@ -644,6 +647,77 @@ xsetsel(char *str)
> setsel(str, CurrentTime);
> }
>
> +char *
> +subprocwd()
> +{
> + struct dirent* dent;
> + DIR* srcdir = opendir("/proc/self/task");
> + char path[PATH_MAX];
> + FILE *fp;
> + int chpid;
> +
> + if (srcdir == NULL)
> + {
> + return NULL;
> + }
> +
> + while((dent = readdir(srcdir)) != NULL)
> + {
> + struct stat st;
> + char newdir[PATH_MAX];
> +
> + if(strcmp(dent->d_name, ".") == 0 || strcmp(dent->d_name, "..") == 0)
> + continue;
> + if (snprintf(newdir, PATH_MAX, "%s/%s", "/proc/self/task",
> dent->d_name) < 0)
> + return NULL;
> + if (stat(newdir, &st) < 0)
> + continue;
> +
> + if (S_ISDIR(st.st_mode)) break;
> + }
> + closedir(srcdir);
> + if (snprintf(path, PATH_MAX, "/proc/self/task/%s/children",
> dent->d_name) < 0)
> + return NULL;
> + if (!(fp = fopen(path, "r")))
> + return NULL;
> + if (fscanf(fp, "%d", &chpid) != 1) {
> + fclose(fp);
> + return NULL;
> + }
> + fclose(fp);
> + if (snprintf(path, PATH_MAX, "/proc/%d/cwd", chpid) < 0)
> + return NULL;
> + return realpath(path, NULL);
> +}
> +

You know, you can have that one cheaper: /proc/self/task will contain a
directory named for the TIDs of each thread of the process. As st is a
single-threaded process, that TID must always equal the PID. So the
first loop can be replaced with.

snprintf(path, PATH_MAX, "/proc/self/task/%d/children", getpid());

Also, your code assumes the shell to be the only child of st, but spawns
other children of st. How about you iterate over all processes, finding
the ones with a PPID equaling st's PID and having an exe pointing to the
shell (maybe remember the shell spawned in a global variable)? Speaking
of remembering things, might it not be easier to just remember the
shell's PID from the point it was spawned in the first place? Then you
don't need to iterate over rarely-available system interfaces.

> +void
> +plumb(char *sel) {
> + if (sel == NULL)
> + return;
> + char *cwd;
> + char *cmd;
> + pid_t child;
> + cwd = subprocwd();
> + if (cwd == NULL)
> + return;
> +

So, if it all failed somehow, you won't even spawn the command in st's
current directory? Nor give any indication of failure to the user.

> + switch(child = fork()) {
> + case -1:
> + return;
> + case 0:
> + if (cwd == NULL)
> + exit(1);
> + cmd = malloc(PATH_MAX+100);
> + if (snprintf(cmd, PATH_MAX+100, "(cd %s ; %s %s)", cwd, plumber_cmd,
> sel) < 0)
> + exit(1);
> + free(cwd);
> + execvp("sh", (char *const []){"sh", "-c", cmd, 0});
> + exit(0);

This won't work if there is a space in cwd. Which there might be. Also,
why change dir in the shell? Just change dir in the process. You have
the directory right there, just call chdir() and call it a day.

Also, do you want word separation to take place? Because if not, I see
no reason for the shell at all.

Ciao,
Markus
Received on Thu Nov 01 2018 - 13:43:18 CET