>As this is my first real execline program, I would like to get some
>feedback, either towards the execline syntax or even towards the
>implementation itself. Thanks in advance.
First thing: please don't ask for code reviews. We don't have the
resources to provide them. I'll do this one because it's short, but if
everyone started to post code snippets for review, we'd never get
anything done. 😄
Second thing: last time I checked, you could not use inotify on
sysfs to watch for file creation. Has it changed? If it has, it's
awesome, but it would only work on recent kernels, so unfortunately
that's not usable on a generic distribution (unless they stick to very
recent kernels).
Third thing: the script is small enough that it's probably worth it
to actually use execline here over a shell, but for both readability
and efficiency you need to trim it down as much as possible.
execline is great in two situations: 1. when you're writing very
simple,
straightforward command lines that don't need many control commands, and
2. when you're generating code automatically (because an execline script
is much simpler to autogenerate than shell). Outside of these two
situations, the point where it is more beneficial to stick to a shell
script arrives _very quickly_, so you want to make your execline scripts
really mean and lean.
All of that said, let's go line by line.
>#!/usr/bin/env -S execlineb
Don't do the env thing in shebangs, except when you want to
distribute a script for platforms you do not control and where you
don't know the installation policies for your interpreter.
"#!/usr/bin/env foo" is only useful when the installation directory for
"foo" is strictly less stable than the one for "env". This is the case
e.g. for scripts meant to be ported to the BSDs when interpreters are
themselves ports. Since some BSDs put ports under /usr/local, the
shebang for interpreters might be #!/usr/local/bin, not #!/usr/bin, and
your script doesn't know, so performing a PATH search via env makes
sense. But for a given Linux distro, or for a script you run for
yourself, you know where your interpreters are (it's likely /usr/bin),
with as much certainty as env, so having an indirection is a net loss.
# check that we have atleast 3 arguments and that the second argument is
"--"
By POSIX convention, -- is an optional separator between options and
arguments; enforcing its existence and position goes against good Unix
practices. If you have a fixed number of arguments before the command
line (here, one), just read that argument, shift, and exec. All the
execline commands work that way, and all chainloading Unix commands
do so, too (e.g. nice).
> importas tty 1
> importas separator 2
> importas count \#
Using the -S or -s option to execlineb avoids doing the
importas/shift/elgetpositionals dance. But if you want to do this dance
anyway, remember to cram all your importas in a multisubstitute { }
block,
that will be more efficient.
># define paths
>define ACTIVE_FILE "/sys/class/tty/tty0/active"
>define TTY_DEVICE "/dev/${tty}"
Same: if you're going to define constants, use a multisubstitute { }
around the defines. It will save some execves and some argv rewriting.
># check everytime the active file changes
>pipeline {
> # trigger this script atleast once
> foreground { echo c $ACTIVE_FILE }
> inotifyd - "${ACTIVE_FILE}:c"
>}
Your first line will definitely be taken into account, but as mentioned
above, I'm not sure inotifyd will ever trigger if $ACTIVE_FILE is on
sysfs.
Also, since your loop isn't using the content of the line but only the
fact that a line exists, you don't need to echo anything fancy for your
first line. Just echo is enough.
># Not sure if this should be parallel
>forstdin -E _event
Can several instances of your command line be run at the same time?
Does it make sense for your goal? Can you make it work without locking?
Is it more efficient? If all the answers are yes, then you should make
it
parallel, otherwise no.
># check if there is nothing already running on the tty
>if -n {
> fdmove 2 1
> redirfd -w 1 /dev/null
> fuser $TTY_DEVICE
>}
And that answers your question: since you only want one thing running
on the tty at a time, you definitely should not make the forstdin
parallel.
Also, you could use fuser -s to avoid the verbosity of redirections.
>exec $_at_
In execline, you don't need "exec". Just $_at_ will exec into the
remainder
of your command line.
The rewritten script:
-----
#!/usr/bin/execlineb -s1
ifelse { test "$#" -lt 2 }
{
foreground { fdmove 1 2 s6-echo -- "$0: usage: $0 tty args..." }
exit 100
}
pipeline
{
foreground { echo }
inotifyd - /sys/class/tty/tty0/active:c
}
forstdin _line
if -n { fuser -s /dev/$1 }
$_at_
-----
execlineb -s1 means that $_at_ will be shifted one notch, which is what
we want. $1 still means the first argument on the original command line,
i.e. the tty.
Since there's no -a anymore (which would lead to UB with POSIX test
which is one of the reasons why I wrote eltest), we can use test here
now. We could also stick to eltest since we're using execline anyway
and there are no builtins. It really doesn't matter.
I use s6-echo (from s6-portable-utils) for the usage message, because
I like to prefix messages with the name of the program - and if $0
starts with a dash, then echo's behaviour is implementation-specific
and difficult to predict. If you prefer, you can use the printf command
instead. I also like to exit 100, by convention, on bad usage, leaving
exit codes like 1, 2, etc. for real failure reports when the command
actually runs.
The shell equivalent to the script would be something like:
-----
#!/bin/sh -e
tty="$1"; shift
if test "$#" -lt 2 ; then
s6-echo -- "$0: usage: $0 tty args..." 1>&2
exit 100
fi
{ echo; inotifyd - /sys/class/tty/tty0/active:c; } | while read ; do
fuser -s "/dev/$tty" || exec "$_at_"
done
-----
but in this case I would say that the execline version is better,
because it only leaves two long-running processes: inotifyd and
forstdin,
whereas the shell leaves three: the parent shell maintaining the
pipeline, inotifyd, and the subshell executing the command.
There you go. HTH,
--
Laurent
Received on Fri May 23 2025 - 13:10:29 CEST