The trap handling is likely fragile

[Earnestly]

Under shells such as dash or mrsh the cleanup function will never be called if sx is interrupted by a signal. The bash (as sh) shell implements a strategy which does ensure it is called but it is the exception.

The only somewhat reliable interpretation of EXIT is that it will be run when the shell script itself exits normally, as if the function was called directly at the bottom of the script. This effectively makes the EXIT trap extraneous.

Due to this more thought is needed to handle signals that could interrupt sx such as INT and TERM especially if the cleanup function wants to call exit.

A potential implementation might be something like this:

cleanup() {
    r=$?

    # See notes below this example code.
    if [ "$r" -gt 128 ]; then
        signal=$((r - 128))
    fi

    if kill -0 "$pid" > /dev/null; then
        # Pass the signal to Xorg(?).  If the return was 0 above then this if
        # condition should not be met and the branch never taken.  If due to
        # any race conditions it is, then the call to kill will resolve to -0
        # and do nothing.
        kill -"$signal" "$pid"
        wait "$pid"
    fi

    if ! stty "$stty"; then
        stty sane
    fi

    xauth remove :"$tty"
}

# ...

for signal in HUP INT TERM; do
    # Resend the signal as they are intended to terminate the script as well.
    trap 'cleanup; trap - "$signal"; kill -s "$signal" "$$"' "$signal"
done

# ...

# Run the cleanup routine unconditionally at the end of the script, replacing
# the role of an EXIT trap.
cleanup

The use of 128 is not portable, and the POSIX standard renders it pointless:

The exit status of a command that terminated because it received a signal shall be reported as greater than 128.

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_02

Historically, shells have returned an exit status of 128+ n, where n represents the signal number. Since signal numbers are not standardized, there is no portable way to determine which signal caused the termination. Also, it is possible for a command to exit with a status in the same range of numbers that the shell would use to report that the command was terminated by a signal. Implementations are encouraged to choose exit values greater than 256 to indicate programs that terminate by a signal so that the exit status cannot be confused with an exit status generated by a normal termination.

https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html#tag_23_02_08

In the end I may simply do the clean up at the end of the script and simply do nothing with signals which will potentially leave Xservers running when they ought not be; either that or port this to bash instead.

[marcthe12]

We could just add HUP INT TERM the existing and ignore the exit codes. The main thing is to kill sxrc or X when completed

[Earnestly]

I've got this commit queued up that I've sat on for awhile which does more or less that:

diff --git a/sx b/sx
index 92facc8..d793b91 100755
--- a/sx
+++ b/sx
@@ -1,21 +1,21 @@
 #!/bin/sh --
 # sx - start an xserver

-# requires xauth Xorg
+# requires xauth Xorg /dev/urandom

 cleanup() {
-    if [ "$pid" ] && kill -0 "$pid" 2> /dev/null; then
-        kill -s TERM "$pid"
-        wait "$pid"
-        r=$?
-    fi
+    for pid; do
+        if kill -0 "$pid" 2> /dev/null; then
+            kill "$pid"
+            wait "$pid"
+        fi
+    done

     if ! stty "$stty"; then
         stty sane
     fi

     xauth remove :"$tty"
-    exit "${r:-$?}"
 }

 stty=$(stty -g)
@@ -29,14 +29,23 @@ mkdir -p -- "$cfgdir" "$datadir"
 export XAUTHORITY="${XAUTHORITY:-$datadir/xauthority}"
 touch -- "$XAUTHORITY"

-trap 'cleanup' EXIT
 xauth add :"$tty" MIT-MAGIC-COOKIE-1 "$(od -An -N16 -tx /dev/urandom | tr -d ' ')"

-# Xorg will check if its SIGUSR1 disposition is SIG_IGN and use this state to
-# reply back to the parent process with its own SIGUSR1 as an indication it is
-# ready to accept connections.
-# Taking advantage of this feature allows us to launch our client directly
-# from a SIGUSR1 handler and avoid the need to poll for server readiness.
-trap 'DISPLAY=:$tty "${@:-$cfgdir/sxrc}"' USR1
-(trap '' USR1 && exec Xorg :"$tty" -keeptty vt"$tty" -noreset -auth "$XAUTHORITY") & pid=$!
-wait "$pid"
+for signal in HUP INT TERM; do
+    # The client variable is set by the USR1 signal trap and contains the
+    # client's PID.
+    # shellcheck disable=SC2154
+    trap 'cleanup "$client" "$server"; trap - "$signal"; kill -s "$signal" "$$"' "$signal"
+done
+trap cleanup EXIT
+
+# Xorg will check if its USR1 disposition is SIG_IGN and use this state to
+# reply back to the parent process with a SIGUSR1 of its own as indication it
+# is ready to accept connections.
+# Taking advantage of this feature allows launching the client directly from a
+# USR1 signal trap which obviates the need to poll for server readiness.
+trap 'DISPLAY=:$tty "${@:-$cfgdir/sxrc}" & client=$!; wait' USR1
+
+(trap '' USR1 && exec Xorg :"$tty" -keeptty vt"$tty" -noreset -auth "$XAUTHORITY") &
+server=$!
+wait

I could push it later on.