Re: [hackers] [dmenu][PATCH] Replace dmenu_run shell with executed application
How about the following then:
#!/bin/sh
${SHELL:-"/bin/sh"} -c "$(dmenu_path | dmenu "$@") & exit"
This doesn't keep shell in processes but at the same time allows to run scripts
10.02.2019, 14:40, "Hiltjo Posthuma" :
> On Sun, Feb 10, 2019 at 11:40:06AM +0100, Leonardo Taccari wrote:
>> Hello Nick,
>>
>> Nick writes:
>> > [...]
>> > Ignore if you're too busy, but why is this considered bad practise?
>> > Is there some case of possible shell escaping or something I'm
>> > failing to see? I just ask for my own education.
>> >
>>
>> (I have no idea if this was original rationale about why not applying
>> this patch but I will try to share why it can be problematic in
>> some cases IME.)
>>
>> According dmenu(1) man page:
>>
>> > dmenu_run is a script used by dwm(1) which lists programs in the user's
>> > $PATH and runs the result in their $SHELL.
>>
>> by using `exec' this is no longer true.
>>
>> The user's $SHELL is no longer used and what can be typed in
>> `dmenu_run' is now restricted, (I don't know how usual is but
>> sometimes I use `|' and other shell commands in dmenu_run).
>
> Exactly this.
>
> It works like this since atleast 2011 (commit
> 56a0d1fa14de915419c037ac2604fe5c5b1fe4a3). If you dig in the git history you
> can see we've had most possible combinations already.
>
> --
> Kind regards,
> Hiltjo
Re: [hackers] [dmenu][PATCH] Replace dmenu_run shell with executed application
On Sun, Feb 10, 2019 at 11:40:06AM +0100, Leonardo Taccari wrote: > Hello Nick, > > Nick writes: > > [...] > > Ignore if you're too busy, but why is this considered bad practise? > > Is there some case of possible shell escaping or something I'm > > failing to see? I just ask for my own education. > > > > (I have no idea if this was original rationale about why not applying > this patch but I will try to share why it can be problematic in > some cases IME.) > > According dmenu(1) man page: > > > dmenu_run is a script used by dwm(1) which lists programs in the user's > > $PATH and runs the result in their $SHELL. > > by using `exec' this is no longer true. > > The user's $SHELL is no longer used and what can be typed in > `dmenu_run' is now restricted, (I don't know how usual is but > sometimes I use `|' and other shell commands in dmenu_run). > Exactly this. It works like this since atleast 2011 (commit 56a0d1fa14de915419c037ac2604fe5c5b1fe4a3). If you dig in the git history you can see we've had most possible combinations already. -- Kind regards, Hiltjo
Re: [hackers] [dmenu][PATCH] Replace dmenu_run shell with executed application
On 10.02.19 11:40, Leonardo Taccari wrote: > The user's $SHELL is no longer used and what can be typed in > `dmenu_run' is now restricted, (I don't know how usual is but > sometimes I use `|' and other shell commands in dmenu_run). Is there a good reason not to explicitly run a shell then? I.e. launch sh -c "printf 'foo' | cat" instead of printf 'foo' | cat and have just the shell executing these commands wait on them instead of the shell executing dmenu? I guess it is a matter of convenience (but there might be something I overlook). For those interested, in my setup I've put the patched script on my path instead of (before) dmenu_run, which works quite nicely to get the behavior I want/like. A good compromise can also be to have both (dmenu_run and dmenu_run_shell). Anyhow, I'll leave it at that and hope some people on this list like the useful way to cut the number of lingering processes in your system down immensely when launching almost everything via dmenu :) If you should happen to decide differently and include the patch just post here and I can help with the man page cleanup.
Re: [hackers] [dmenu][PATCH] Replace dmenu_run shell with executed application
Hello Nick, Nick writes: > [...] > Ignore if you're too busy, but why is this considered bad practise? > Is there some case of possible shell escaping or something I'm > failing to see? I just ask for my own education. > (I have no idea if this was original rationale about why not applying this patch but I will try to share why it can be problematic in some cases IME.) According dmenu(1) man page: > dmenu_run is a script used by dwm(1) which lists programs in the user's > $PATH and runs the result in their $SHELL. by using `exec' this is no longer true. The user's $SHELL is no longer used and what can be typed in `dmenu_run' is now restricted, (I don't know how usual is but sometimes I use `|' and other shell commands in dmenu_run).
Re: [hackers] [dmenu][PATCH] Replace dmenu_run shell with executed application
On February 9, 2019 7:48:34 PM GMT+00:00, Nick wrote: >Quoth Hiltjo Posthuma: >> On Sat, Feb 09, 2019 at 01:48:34PM +0100, Jan Bessai wrote: >> > On Sun, Dec 30, 2018 at 02:59:13PM +0100, Jan Bessai wrote: >> > > Thanks! I've attached the updated patch below. >> > >> > Sorry if I'm breaching any rules, but any update on >accepting/rejecting >> > the patch? >> > >> >> Rejected > >Ignore if you're too busy, but why is this considered bad practise? >Is there some case of possible shell escaping or something I'm >failing to see? I just ask for my own education. I would also be interested in the reason for the rejection. It seems sensible to me to not let all these shells linger around for no reason. Cheers, Silvan
Re: [hackers] [dmenu][PATCH] Replace dmenu_run shell with executed application
Quoth Hiltjo Posthuma: > On Sat, Feb 09, 2019 at 01:48:34PM +0100, Jan Bessai wrote: > > On Sun, Dec 30, 2018 at 02:59:13PM +0100, Jan Bessai wrote: > > > Thanks! I've attached the updated patch below. > > > > Sorry if I'm breaching any rules, but any update on accepting/rejecting > > the patch? > > > > Rejected Ignore if you're too busy, but why is this considered bad practise? Is there some case of possible shell escaping or something I'm failing to see? I just ask for my own education.
Re: [hackers] [dmenu][PATCH] Replace dmenu_run shell with executed application
On Sat, Feb 09, 2019 at 01:48:34PM +0100, Jan Bessai wrote: > On Sun, Dec 30, 2018 at 02:59:13PM +0100, Jan Bessai wrote: > > Thanks! I've attached the updated patch below. > > Sorry if I'm breaching any rules, but any update on accepting/rejecting > the patch? > Rejected -- Kind regards, Hiltjo
Re: [hackers] [dmenu][PATCH] Replace dmenu_run shell with executed application
On Sun, Dec 30, 2018 at 02:59:13PM +0100, Jan Bessai wrote: > Thanks! I've attached the updated patch below. Sorry if I'm breaching any rules, but any update on accepting/rejecting the patch?
Re: [hackers] [dmenu][PATCH] Replace dmenu_run shell with executed application
[2018-12-30 14:45] Silvan Jegen
> To keep this portable you should refrain from using backticks (`) and
> use the POSIX-way
Thanks! I've attached the updated patch below.
---
dmenu_run | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dmenu_run b/dmenu_run
index 834ede5..1eff6af 100755
--- a/dmenu_run
+++ b/dmenu_run
@@ -1,2 +1,2 @@
#!/bin/sh
-dmenu_path | dmenu "$@" | ${SHELL:-"/bin/sh"} &
+exec $(dmenu_path | dmenu "$@")
--
2.20.1
Re: [hackers] [dmenu][PATCH] Replace dmenu_run shell with executed application
Hi
[2018-12-30 13:41] Jan Bessai
>
> Currently dmenu_run spawns a subshell and keeps running for each process
> it executes. Over time this litters up the process list with useless
> instances of dmenu_run, which do nothing but wait for their child to
> exit. The patch below replaces the dmenu_run process with its child,
> freeing up resources immediately. The difference is especially
> noticeable when dmenu is used in window managers.
>
> -- Jan
>
> ---
> dmenu_run | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dmenu_run b/dmenu_run
> index 834ede5..5c9b4e8 100755
> --- a/dmenu_run
> +++ b/dmenu_run
> @@ -1,2 +1,2 @@
> #!/bin/sh
> -dmenu_path | dmenu "$@" | ${SHELL:-"/bin/sh"} &
> +exec `dmenu_path | dmenu "$@"`
To keep this portable you should refrain from using backticks (`) and
use the POSIX-way:
exec $(dmenu_path | dmenu "$@")
Cheers,
Silvan
