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 16:12:40 +0100

On 01/11/2018 15:04, John Soros wrote:
> 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
>
Yes! This is much, much better! Thanks. Pity that it still doesn't work
on OpenBSD.
 config.def.h | 6 ++++++
 st.c | 9 +++++++++
 st.h | 2 ++
 x.c | 29 +++++++++++++++++++++++++++++
 4 files changed, 46 insertions(+)

diff --git a/config.def.h b/config.def.h
index 87ebdbb..b4f9b9a 100644
--- a/config.def.h
+++ b/config.def.h
_AT_@ -467,3 +467,9 @@ static char ascii_printable[] =
         " !\"#$%&'()*+,-./0123456789:;<=>?"
         "_AT_ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_"
         "`abcdefghijklmnopqrstuvwxyz{|}~";
+
+/*
+ * plumb_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 *plumb_cmd = "plumb";
diff --git a/st.c b/st.c
index 3607ff1..ab7e78f 100644
--- a/st.c
+++ b/st.c
_AT_@ -235,6 +235,15 @@ static uchar utfmask[UTF_SIZ + 1] = {0xC0, 0x80,
0xE0, 0xF0, 0xF8};
 static Rune utfmin[UTF_SIZ + 1] = { 0, 0, 0x80, 0x800,
0x10000};
 static Rune utfmax[UTF_SIZ + 1] = {0x10FFFF, 0x7F, 0x7FF, 0xFFFF,
0x10FFFF};

+char *
+subprocwd(void)
+{
+ char path[PATH_MAX];
+ if (snprintf(path, PATH_MAX, "/proc/%d/cwd", pid) < 0)
+ return NULL;
+ return realpath(path, NULL);
+}
+
 ssize_t
 xwrite(int fd, const char *s, size_t len)
 {
diff --git a/st.h b/st.h
index 0f2db33..eb53f49 100644
--- a/st.h
+++ b/st.h
_AT_@ -125,6 +125,8 @@ char *xstrdup(char *);
 void kscrolldown(const Arg *);
 void kscrollup(const Arg *);

+char *subprocwd(void);
+
 /* config.h globals */
 extern char *utmp;
 extern char *stty_args;
diff --git a/x.c b/x.c
index ce255ba..00e7db3 100644
--- a/x.c
+++ b/x.c
_AT_@ -5,6 +5,7 @@
 #include <locale.h>
 #include <signal.h>
 #include <sys/select.h>
+#include <sys/wait.h>
 #include <time.h>
 #include <unistd.h>
 #include <libgen.h>
_AT_@ -645,6 +646,32 @@ xsetsel(char *str)
         setsel(str, CurrentTime);
 }

+void
+plumb(char *sel) {
+ if (sel == NULL)
+ return;
+ char *cwd;
+ pid_t child;
+ cwd = subprocwd();
+ if (cwd == NULL)
+ return;
+
+ switch(child = fork()) {
+ case -1:
+ return;
+ case 0:
+ if (chdir(cwd) != 0) {
+ free(cwd);
+ exit(1);
+ }
+ free(cwd);
+ execvp(plumb_cmd, (char *const []){plumb_cmd, sel, 0});
+ exit(0);
+ default:
+ waitpid(child, NULL, 0);
+ }
+}
+
 void
 brelease(XEvent *e)
 {
_AT_@ -657,6 +684,8 @@ brelease(XEvent *e)
                 selpaste(NULL);
         else if (e->xbutton.button == Button1)
                 mousesel(e, 1);
+ else if (e->xbutton.button == Button3)
+ plumb(xsel.primary);
 }

 void
Received on Thu Nov 01 2018 - 16:12:40 CET