Should these be $_ret?

This should have some comment explaining what is going on.

I think you only specify the variable name without $ if it's inside $((...)).

I see that we use both styles in the tree.

It is recommended by shellcheck to let $(()) expand variables on its own. $? has to be used as a variable in this case, however, because just ? has a different meaning in this context.

Both $(($foobar)) and $((foobar)) is valid sh.

Are you sure that rc scripts are always run in a separate shell? I have some vague notion that it's possible to source them, and maybe there are some modes where rc or service do that. Maybe I'm just completely wrong?

This is a good question.

Here's a fragment of run_rc_script():

			if [ -n "$rc_boottrace" ]; then
				boottrace_fn "$_file" "$_arg"
			elif [ -n "$rc_fast_and_loose" ]; then
				set $_arg; . $_file
			else
				( trap "echo Script $_file interrupted >&2 ; kill -QUIT $$" 3
				  trap "echo Script $_file interrupted >&2 ; exit 1" 2
				  trap "echo Script $_file running >&2" 29
				  set $_arg; . $_file )
			fi

rc_fast_and_loose seems to just source everything into the current shell. It might be that other programs like service(8) do something similar. I've not checked.

However, a quick grep over rc.d for exit reveals that rc scripts use exit a lot. My first thought is that rc_fast_and_loose and similar use cases are just not really working at the moment.

For reference, service(8) only handles one service script at a time so it does not care about exits in those scripts.

Perhaps we should just leave the exit status set. Then, if the script is run in its own shell, it'll exit with the correct status, and if it is sourced into an existing shell, it will not trigger an exit.

Alternately, perhaps rc_fast_and_loose is broken and should be removed. I'm not sure.

If I understand correctly, setting the exit status without calling exiting the whole script could be done as follows:

(exit "$_ret")

instead of

exit "$_ret"

This way we exit the subshell but still set the exit status as long as (exit ...) is the last command in the script.

As a side note: I do believe that rc_fast_and_loose is broken currently.

I think that should work, yes.

I don't insist on doing that. As you say, there are other scripts that invoke exit already.

I would just suggest giving this change a long MFC timeout, so that there's time to catch bugs before the change appears on a stable branch (if at all).

Here's a related thread from 2012 which suggests that rc_fast_and_loose is broken: https://lists.freebsd.org/pipermail/freebsd-current/2012-January/031167.html

Also, I think we can use return 1 instead of (exit 1) actually. It's just that calling return outside of a function is not supported by POSIX I think. At the same time, rc scripts are very FreeBSD-specific and our sh(1) supports calling return like that.

I'd rather not rely on non-POSIX sh behaviour here, unless there's already a lot of precedent for that in existing rc scripts.