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

From: John Soros <j_AT_roxor.me>
Date: Thu, 1 Nov 2018 15:04:41 +0100

On 01/11/2018 13:43, Markus Wichmann wrote:
> 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
>

Thanks for the comments and advise. I will try to implement them soon.
As far as I can see, there is no solution for this on OpenBSD.
Regs,
John
Received on Thu Nov 01 2018 - 15:04:41 CET