In D57213#1313293, @des wrote:

Please don't, I need these tests to remain portable.

I suggest discussing this on a mailing list.

It would be best to test this in practice with some jail images first, to see that practical applications can run without the shell package.

Is signal(3) the right reference? C code had better use sigaction(2) instead, or the list might be moved to a new signal(7) page?

In D55038#1306921, @bnovkov wrote:

In D55038#1306915, @jilles wrote:

With the existing rlimit functionality specified by POSIX, I can, as a user, defend against a forkbomb or prevent forks/kqueues entirely in a particular process subtree. A system administrator can also set resource limits which will be applied at login or service start.

It feels a little weird to clutter up ulimit -a and limits with a limit for this pretty exotic feature.

With the existing rlimit functionality specified by POSIX, I can, as a user, defend against a forkbomb or prevent forks/kqueues entirely in a particular process subtree. A system administrator can also set resource limits which will be applied at login or service start. It seems like that setting limits on a particular process subtree will not work any more.

The #! mechanism is already described (more briefly) in lib/libsys/execve.2 lines 73...100. That duplication is certainly undesirable.

The sh part looks good and appears to work in a quick test.

In D55191#1296317, @des wrote:

In D55191#1295909, @bdrewery wrote:

read -t 0.250 isn't supported. Best we can do is 1 second. One of the reads does not time out while the other does. So this change reduces 2 seconds of sleep in the test suite.

Fix assertions; correct assertion

Let's not mention the behaviour when return exits the shell during an EXIT trap action, since it's obscure and probably incorrect (I think it's supposed to work like exit, with a difference between return by itself and return "$?", but in practice it always ignores the argument during an EXIT trap action).

In D55295#1264736, @ziaee wrote:

Maybe we should describe it as 'default' file for user's history? Since it can be overwritten by HISTFILE.

In D55191#1261862, @des wrote:

Since the test assumes a 250 ms response time, the test delay can be 1 second instead of 3.

In D55023#1257720, @rmacklem wrote:

In D55023#1257715, @kib wrote:

BTW, chmod(1), chflags(1), ls(1) and find(1) consistently use -P for the same functionality. ls(1) does not fail if the argument is not a symbolic link.

One apparent mistake, looks OK otherwise.

In D53499#1221677, @bdrewery wrote:

I'm only intending this to target $(alias) and $(alias --). I don't think it would be common but I've at least written some code that duplicates some aliases with different handling if they exist, and found $(alias) forking unexpectedly.

This is correct, and useful if there is a practical use case that frequently does $(alias) (but not $(alias NAME); a little more code would be needed). Please check the use cases so we add the right amount of logic here.

In D52527#1199611, @kevans wrote:

I did wonder for a second if the /rescue variants of shells do or should assume setugid behavior and avoid loading dotfiles in case something in the profile or other bits finds a way to make the shell unusable, but I didn't really convince myself.

It seems reasonable to me to use .profile and other files from root's home directory in single user mode as well. Having no .profile at all in single user mode would be bad. I don't think special logic in pkg for hard links is worth it for this particular special case.

In D49201#1122836, @pstef wrote:

In D49201#1122784, @jilles wrote:

This adds a non-POSIX feature (although it is pretty common); is the goal to make sh a mostly feature-complete alternative to bash?

This adds a non-POSIX feature (although it is pretty common); is the goal to make sh a mostly feature-complete alternative to bash?

Given that set -e is a controversial issue (see the disagreement here and in the "Conclusions" section of https://mywiki.wooledge.org/BashFAQ/105 ), I don't think the man page should recommend anything about it, although it should mention some pitfalls in a neutral manner.

The immediately preceding sentence about shell functions is an example of unexpected behaviour of set -e. The part "all commands of the function are considered to be tested as well" may result in code unexpectedly executing in the function and the function returning a different return status.

This looks like a reasonable quick fix, but I think it's unfortunate the /root directory permissions are specified both in bsd.dirs.mk variables and in the BSD.root.dist mtree file. It looks like the bsd.dirs.mk variables are required because of pkgbase.

I agree with the analysis, but the variables should still be defined inside main (with static) so they cannot be used outside. Some of the other functions in this file have their own smark locals that should not be mixed up with main's.

To me, it looks like that situation can't actually happen. If there is an error or SIGINT during early initialization, state will still be 0 and the if (setjmp(main_handler.loc)) block will immediately call exitshell(exitstatus) without touching smark. If I'm wrong, please provide a reproduction scenario.

This doesn't follow the Don't Repeat Yourself principle, but currently I don't have a better idea how to express this code.

Code is OK like this.

Adding support for escape sequences might make it important to increase PROMPTLEN.

In D37620#855668, @corvink wrote:

Thanks for the hint. kill %+ and jobs -p %+ are already working. So, no patch required. Might be a good idea to mention it in the man page.

How do other shells with a similar option handle this? For example, zsh has a TRAPS_ASYNC option, but does not document how to access the interrupted job in the trap action.

In hindsight, this is how it should have been in the first place, since it's the same thing Linux distributions do. They (used to?) place *.csh files too for sourcing from /etc/csh.login.

Since a similar effect can be obtained by having the shell ignore the signal via trap '' TTOU, I'm not sure it's a good idea to add an option for this.