Skip to content

Commit

Permalink
kernel: Use persistent term to store os_cmd_shell value
Browse files Browse the repository at this point in the history
This has two purposes:

1. Make it impossible to change the value through the application:set_env API.
2. Make the lookup slightly faster.
  • Loading branch information
garazdawi committed Nov 4, 2024
1 parent 9b9992d commit bb27085
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 21 deletions.
21 changes: 12 additions & 9 deletions lib/kernel/src/os.erl
Original file line number Diff line number Diff line change
Expand Up @@ -587,14 +587,16 @@ get_option(Opt, Options, Default) ->
_ -> throw(badopt)
end.

-define(KERNEL_OS_CMD_SHELL_KEY, kernel_os_cmd_shell).

mk_cmd({win32,_}, Cmd) ->
{ok, Shell} = application:get_env(kernel, os_cmd_shell),
Shell = persistent_term:get(?KERNEL_OS_CMD_SHELL_KEY),
Command = lists:concat([Shell, " /c", Cmd]),
{Command, [], [], <<>>};
mk_cmd(_,Cmd) ->
%% Have to send command in like this in order to make sh commands like
%% cd and ulimit available.
{ok, Shell} = application:get_env(kernel, os_cmd_shell),
Shell = persistent_term:get(?KERNEL_OS_CMD_SHELL_KEY),
{Shell ++ " -s unix:cmd", [out],
%% We insert a new line after the command, in case the command
%% contains a comment character.
Expand All @@ -615,13 +617,14 @@ mk_cmd(_,Cmd) ->

-doc false.
internal_init_cmd_shell() ->
case application:get_env(kernel, os_cmd_shell) of
undefined ->
application:set_env(kernel, os_cmd_shell,
internal_init_cmd_shell(os:type()));
_ ->
ok
end.
Shell =
case application:get_env(kernel, os_cmd_shell) of
undefined ->
internal_init_cmd_shell(os:type());
{ok, Val} ->
Val
end,
persistent_term:put(?KERNEL_OS_CMD_SHELL_KEY, Shell).
internal_init_cmd_shell({win32,Wtype}) ->
case {os:getenv("COMSPEC"),Wtype} of
{false,windows} -> "command.com";
Expand Down
18 changes: 6 additions & 12 deletions lib/kernel/test/os_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -469,21 +469,15 @@ error_info(Config) ->
],
error_info_lib:test_error_info(os, L).

os_cmd_shell(Config) ->
DataDir = proplists:get_value(data_dir, Config),
SysShell = filename:join(DataDir, "sys_shell"),
%% Check that is *not* possible to change shell after startup
os_cmd_shell(_Config) ->

{ok, OldShell} = application:get_env(kernel, os_cmd_shell),
try
application:set_env(kernel, os_cmd_shell, SysShell),
application:set_env(kernel, os_cmd_shell, "broken shell"),

%% os:cmd should not try to detect the shell location rather than use
%% the value from kernel:os_cmd_shell parameter
comp("sys_shell", os:cmd("ls"))
after
application:set_env(kernel, os_cmd_shell, OldShell)
end.
%% os:cmd should continue to work as normal
comp("hello", os:cmd("echo hello")).

%% When started with os_cmd_shell set, we make sure that it is used.
os_cmd_shell_peer(Config) ->
DataDir = proplists:get_value(data_dir, Config),
SysShell = "\"" ++ filename:join(DataDir, "sys_shell") ++ "\"",
Expand Down

0 comments on commit bb27085

Please sign in to comment.