File 0514-kernel-Fix-os-cmd-error-report-for-open_port-errors.patch of Package erlang
From 10a928195c26a59249cb36a8f3e556fc39593f8e Mon Sep 17 00:00:00 2001
From: Lukas Larsson <lukas@erlang.org>
Date: Wed, 19 Oct 2022 14:11:06 +0200
Subject: [PATCH] kernel: Fix os:cmd error report for open_port errors
The error_info code for os:cmd inadvertenly hid errors
when open_port failed to start due to some reason, for
example out of file descriptors.
So we fix os:cmd and add tests to make sure that it works.
---
lib/kernel/src/erl_kernel_errors.erl | 16 +++-
lib/kernel/src/os.erl | 56 ++++++++-----
lib/kernel/test/Makefile | 10 ++-
lib/kernel/test/error_info_lib.erl | 115 ---------------------------
lib/kernel/test/os_SUITE.erl | 46 ++++++++++-
lib/stdlib/test/error_info_lib.erl | 18 ++++-
6 files changed, 115 insertions(+), 146 deletions(-)
delete mode 100644 lib/kernel/test/error_info_lib.erl
diff --git a/lib/kernel/src/erl_kernel_errors.erl b/lib/kernel/src/erl_kernel_errors.erl
index 376ce31f16..6c03047323 100644
--- a/lib/kernel/src/erl_kernel_errors.erl
+++ b/lib/kernel/src/erl_kernel_errors.erl
@@ -42,6 +42,8 @@ format_error(_Reason, [{M,F,As,Info}|_]) ->
format_erl_ddll_error(_, _, _) ->
[].
+format_os_error(cmd, _, {open_port, Reason}) ->
+ [{general, maybe_posix_message(Reason)}];
format_os_error(cmd, [_], _) ->
[not_charlist];
format_os_error(cmd, [_, _], Cause) ->
@@ -71,6 +73,14 @@ format_os_error(unsetenv, [Name], _) ->
format_os_error(_, _, _) ->
[].
+maybe_posix_message(Reason) ->
+ case erl_posix_msg:message(Reason) of
+ "unknown POSIX error" ->
+ io_lib:format("open_port failed with reason: ~tp",[Reason]);
+ PosixStr ->
+ io_lib:format("~ts (~tp)",[PosixStr, Reason])
+ end.
+
must_be_atom(Term, Default) when is_atom(Term) -> Default;
must_be_atom(_, _) -> not_atom.
@@ -129,6 +139,8 @@ must_be_env_charlist(_) ->
format_error_map([""|Es], ArgNum, Map) ->
format_error_map(Es, ArgNum + 1, Map);
+format_error_map([{general, E}|Es], ArgNum, Map) ->
+ format_error_map(Es, ArgNum, Map#{ general => expand_error(E)});
format_error_map([E|Es], ArgNum, Map) ->
format_error_map(Es, ArgNum + 1, Map#{ArgNum => expand_error(E)});
format_error_map([], _, Map) ->
@@ -155,4 +167,6 @@ expand_error(not_list) ->
expand_error(not_map) ->
<<"not a map">>;
expand_error(not_proper_list) ->
- <<"not a proper list">>.
+ <<"not a proper list">>;
+expand_error(Other) ->
+ Other.
diff --git a/lib/kernel/src/os.erl b/lib/kernel/src/os.erl
index f4c5d01b05..dc73ca1556 100644
--- a/lib/kernel/src/os.erl
+++ b/lib/kernel/src/os.erl
@@ -258,9 +258,11 @@ extensions() ->
Command :: os_command().
cmd(Cmd) ->
try
- cmd(Cmd, #{ })
+ do_cmd(Cmd, #{ })
catch
- error:_ ->
+ throw:{open_port, Reason} ->
+ badarg_with_cause([Cmd], {open_port, Reason});
+ throw:badarg ->
badarg_with_info([Cmd])
end.
@@ -273,15 +275,20 @@ cmd(Cmd, Opts) ->
catch
throw:badopt ->
badarg_with_cause([Cmd, Opts], badopt);
- error:_ ->
+ throw:{open_port, Reason} ->
+ badarg_with_cause([Cmd, Opts], {open_port, Reason});
+ throw:badarg ->
badarg_with_info([Cmd, Opts])
end.
do_cmd(Cmd, Opts) ->
MaxSize = get_option(max_size, Opts, infinity),
{SpawnCmd, SpawnOpts, SpawnInput, Eot} = mk_cmd(os:type(), validate(Cmd)),
- Port = open_port({spawn, SpawnCmd}, [binary, stderr_to_stdout,
- stream, in, hide | SpawnOpts]),
+ Port = try open_port({spawn, SpawnCmd}, [binary, stderr_to_stdout,
+ stream, in, hide | SpawnOpts])
+ catch error:Reason ->
+ throw({open_port, Reason})
+ end,
MonRef = erlang:monitor(port, Port),
true = port_command(Port, SpawnInput),
Bytes = get_data(Port, MonRef, Eot, [], 0, MaxSize),
@@ -346,34 +353,39 @@ mk_cmd(_,Cmd) ->
["(", unicode:characters_to_binary(Cmd), "\n) </dev/null; echo \"\^D\"\n"],
<<$\^D>>}.
-validate(Atom) when is_atom(Atom) ->
- validate(atom_to_list(Atom));
-validate(List) when is_list(List) ->
- case validate1(List) of
+validate(Term) ->
+ try validate1(Term)
+ catch error:_ -> throw(badarg)
+ end.
+
+validate1(Atom) when is_atom(Atom) ->
+ validate1(atom_to_list(Atom));
+validate1(List) when is_list(List) ->
+ case validate2(List) of
false ->
List;
- true ->
+ true ->
%% Had zeros at end; remove them...
string:trim(List, trailing, [0])
end.
-validate1([0|Rest]) ->
+validate2([0|Rest]) ->
+ validate3(Rest);
+validate2([C|Rest]) when is_integer(C), C > 0 ->
validate2(Rest);
-validate1([C|Rest]) when is_integer(C), C > 0 ->
- validate1(Rest);
-validate1([List|Rest]) when is_list(List) ->
- validate1(List) or validate1(Rest);
-validate1([]) ->
+validate2([List|Rest]) when is_list(List) ->
+ validate2(List) or validate2(Rest);
+validate2([]) ->
false.
%% Ensure that the rest is zero only...
-validate2([]) ->
+validate3([]) ->
true;
-validate2([0|Rest]) ->
- validate2(Rest);
-validate2([List|Rest]) when is_list(List) ->
- validate2(List),
- validate2(Rest).
+validate3([0|Rest]) ->
+ validate3(Rest);
+validate3([List|Rest]) when is_list(List) ->
+ validate3(List),
+ validate3(Rest).
get_data(Port, MonRef, Eot, Sofar, Size, Max) ->
receive
diff --git a/lib/kernel/test/Makefile b/lib/kernel/test/Makefile
index 130e626b56..f7776f44fe 100644
--- a/lib/kernel/test/Makefile
+++ b/lib/kernel/test/Makefile
@@ -70,7 +70,6 @@ MODULES= \
erl_prim_loader_SUITE \
erl_uds_dist \
error_handler_SUITE \
- error_info_lib \
error_logger_SUITE \
error_logger_warn_SUITE \
file_SUITE \
@@ -132,7 +132,11 @@ APP_FILES = \
topApp2.app \
topApp3.app
-ERL_FILES= $(MODULES:%=%.erl) code_a_test.erl
+STDLIB_MODULES= error_info_lib
+
+ERL_FILES= $(MODULES:%=%.erl) code_a_test.erl \
+ $(STDLIB_MODULES:%=$(ERL_TOP)/lib/stdlib/test/%.erl)
+
HRL_FILES= \
kernel_test_lib.hrl \
socket_test_evaluator.hrl \
@@ -176,7 +180,8 @@ erl_uds_dist.erl: ../examples/erl_uds_di
make_emakefile: $(ERL_FILES)
$(ERL_TOP)/make/make_emakefile $(ERL_COMPILE_FLAGS) -o$(EBIN) '*_SUITE_make' \
> $(EMAKEFILE)
- $(ERL_TOP)/make/make_emakefile $(ERL_COMPILE_FLAGS) -o$(EBIN) $(MODULES) \
+ $(ERL_TOP)/make/make_emakefile $(ERL_COMPILE_FLAGS) -o$(EBIN) \
+ $(MODULES) $(STDLIB_MODULES) \
>> $(EMAKEFILE)
tests debug opt: make_emakefile
diff --git a/lib/kernel/test/error_info_lib.erl b/lib/kernel/test/error_info_lib.erl
deleted file mode 100644
index d1afaf4589..0000000000
--- a/lib/kernel/test/error_info_lib.erl
+++ /dev/null
@@ -1,115 +0,0 @@
-%%
-%% %CopyrightBegin%
-%%
-%% Copyright Ericsson AB 2021. All Rights Reserved.
-%%
-%% Licensed under the Apache License, Version 2.0 (the "License");
-%% you may not use this file except in compliance with the License.
-%% You may obtain a copy of the License at
-%%
-%% http://www.apache.org/licenses/LICENSE-2.0
-%%
-%% Unless required by applicable law or agreed to in writing, software
-%% distributed under the License is distributed on an "AS IS" BASIS,
-%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-%% See the License for the specific language governing permissions and
-%% limitations under the License.
-%%
-%% %CopyrightEnd%
-%%
--module(error_info_lib).
--export([test_error_info/2, test_error_info/3]).
-
-test_error_info(Module, List) ->
- test_error_info(Module, List, []).
-
-test_error_info(Module, L0, Options) ->
- L1 = lists:foldl(fun({_,A}, Acc) when is_integer(A) -> Acc;
- ({F,A}, Acc) -> [{F,A,[]}|Acc];
- ({F,A,Opts}, Acc) -> [{F,A,Opts}|Acc]
- end, [], L0),
- Tests = ordsets:from_list([{F,length(A)} || {F,A,_} <- L1] ++
- [{F,A} || {F,A} <- L0, is_integer(A)]),
- Bifs0 = get_bifs(Module, Options),
- Bifs = ordsets:from_list(Bifs0),
- NYI = [{F,lists:duplicate(A, '*'),nyi} || {F,A} <- Bifs -- Tests],
- L = lists:sort(NYI ++ L1),
- do_error_info(L, Module, []).
-
-get_bifs(Module, Options) ->
- case lists:member(snifs_only, Options) of
- true ->
- [{F,A} || {M,F,A} <- erlang:system_info(snifs),
- M =:= Module,
- A =/= 0];
- false ->
- [{F,A} || {F,A} <- Module:module_info(exports),
- A =/= 0,
- F =/= module_info]
- end.
-
-do_error_info([{_,Args,nyi}=H|T], Module, Errors) ->
- case lists:all(fun(A) -> A =:= '*' end, Args) of
- true ->
- do_error_info(T, Module, [{nyi,H}|Errors]);
- false ->
- do_error_info(T, Module, [{bad_nyi,H}|Errors])
- end;
-do_error_info([{F,Args,Opts}|T], Module, Errors) ->
- eval_bif_error(F, Args, Opts, T, Module, Errors);
-do_error_info([], _Module, Errors0) ->
- case lists:sort(Errors0) of
- [] ->
- ok;
- [_|_]=Errors ->
- io:format("\n~p\n", [Errors]),
- ct:fail({length(Errors),errors})
- end.
-
-eval_bif_error(F, Args, Opts, T, Module, Errors0) ->
- try apply(Module, F, Args) of
- Result ->
- case lists:member(no_fail, Opts) of
- true ->
- do_error_info(T, Module, Errors0);
- false ->
- do_error_info(T, Module, [{should_fail,{F,Args},Result}|Errors0])
- end
- catch
- error:Reason:Stk ->
- SF = fun(Mod, _, _) -> Mod =:= test_server end,
- Str = erl_error:format_exception(error, Reason, Stk, #{stack_trim_fun => SF}),
- BinStr = iolist_to_binary(Str),
- ArgStr = lists:join(", ", [io_lib:format("~p", [A]) || A <- Args]),
- io:format("\n~p:~p(~s)\n~ts", [Module,F,ArgStr,BinStr]),
-
- case Stk of
- [{Module,ActualF,ActualArgs,Info}|_] ->
- RE = <<"[*][*][*] argument \\d+:">>,
- Errors1 = case re:run(BinStr, RE, [{capture, none}]) of
- match ->
- Errors0;
- nomatch when Reason =:= system_limit ->
- Errors0;
- nomatch ->
- case lists:member(unexplained, Opts) of
- true ->
- Errors0;
- false ->
- [{no_explanation,{F,Args},Info}|Errors0]
- end
- end,
-
- Errors = case {ActualF,ActualArgs} of
- {F,Args} ->
- Errors1;
- _ ->
- [{renamed,{F,length(Args)},{ActualF,ActualArgs}}|Errors1]
- end,
-
- do_error_info(T, Module, Errors);
- _ ->
- Errors = [{renamed,{F,length(Args)},hd(Stk)}|Errors0],
- do_error_info(T, Module, Errors)
- end
- end.
diff --git a/lib/kernel/test/os_SUITE.erl b/lib/kernel/test/os_SUITE.erl
index a84acf3b34..fbae6892f6 100644
--- a/lib/kernel/test/os_SUITE.erl
+++ b/lib/kernel/test/os_SUITE.erl
@@ -396,10 +396,48 @@ do_perf_counter_test(CntArgs, Conv, Upper, Lower, Iters) ->
do_perf_counter_test(CntArgs, Conv, Upper, Lower, Iters-1)
end.
-error_info(_Config) ->
- L = [{cmd, [{no,string}]},
+error_info(Config) ->
+
+
+ ExhaustFDs =
+ fun(M,F,A) ->
+ case os:type() of
+ {unix, _} ->
+ {ok, Peer, Node} = ?CT_PEER(),
+ FN = filename:join(
+ proplists:get_value(priv_dir, Config),
+ "error_info"),
+ try
+ erpc:call(
+ Node,
+ fun() ->
+ io:format("Starting to open files..."),
+ (fun FDs(N) ->
+ case file:open(FN, [write]) of
+ {ok, _ } -> FDs(N+1);
+ {error, _} ->
+ io:format("Opened ~p files",[N])
+ end
+ end)(0),
+ apply(M,F,A)
+ end)
+ catch error:{exception, ErrorReason, StackTrace} ->
+ erlang:raise(error, ErrorReason, StackTrace)
+ after
+ peer:stop(Peer)
+ end;
+ _ ->
+ apply(M,F,A)
+ end
+ end,
+
+ L = [{cmd, [{no, string}]},
+ {cmd, [["echo 1",0,0,0,1]]},
{cmd, [{no, string}, #{}]},
{cmd, [{no, string}, no_map]},
+ {cmd, ["echo 1"], [{general, "too many open files \\(emfile\\)"},
+ {wrapper, ExhaustFDs}] ++
+ [no_fail || win32 =:= element(1, os:type())]},
{find_executable, 1}, %Not a BIF.
{find_executable, 2}, %Not a BIF.
@@ -416,7 +454,7 @@ error_info(_Config) ->
{perf_counter,[bad_time_unit]},
- {putenv, [<<"bad_key">>, <<"bad_value">>]},
+ {putenv, [<<"bad_key">>, <<"bad_value">>],[{1,".*"},{2,".*"}]},
{putenv, ["key", <<"bad_value">>]},
{putenv, [<<"bad_key">>, "value"]},
{putenv, ["abc=", "xyz"]},
@@ -424,7 +462,7 @@ error_info(_Config) ->
{set_signal, [{bad,signal}, ignore]},
{set_signal, [{bad,signal}, ignore]},
{set_signal, [bad_signal, bad_handling]},
- {set_signal, [{bad,signal}, bad_handling]},
+ {set_signal, [{bad,signal}, bad_handling],[{1,".*"},{2,".*"}]},
{system_time, [bad_time_unit]},
{unsetenv, [{bad,key}]}
diff --git a/lib/stdlib/test/error_info_lib.erl b/lib/stdlib/test/error_info_lib.erl
index 716fa47eb9..33b7ae4729 100644
--- a/lib/stdlib/test/error_info_lib.erl
+++ b/lib/stdlib/test/error_info_lib.erl
@@ -20,6 +20,21 @@
-module(error_info_lib).
-export([test_error_info/2, test_error_info/3]).
+%% The wrapper fun should behave as if it was apply/3.
+%% See os_SUITE for an example usage.
+-type wrapper() :: fun((module(),function(),list(term)) -> term()).
+
+%% Options that can be given to testcases
+-type option() :: {integer(), Regexp :: string()} | %% Match argument #1 against RegExp
+ {general, Regexp :: string()} | %% Match general info against RegExp
+ {wrapper, wrapper()} | %% Wrap the test call using this fun
+ {gl, pid()} | %% Use this group leader for the test
+ no_fail | %% The test will not fail
+ allow_rename | %% Allow the exception to not originate from Func
+ unexplained. %% Allow the test to not provide any explanation
+-type test() :: {Func :: function(), Args :: [term()]} |
+ {Func :: function(), Args :: [term()], Opts :: list(option())}.
+-spec test_error_info(module(), list(test())) -> ok.
test_error_info(Module, List) ->
test_error_info(Module, List, []).
@@ -75,7 +90,8 @@ do_error_info([], _Module, Errors0) ->
eval_bif_error(F, Args, Opts, T, Module, Errors0) ->
OldGl = group_leader(),
group_leader(proplists:get_value(gl, Opts, OldGl), self()),
- try apply(Module, F, Args) of
+ Wrapper = proplists:get_value(wrapper, Opts, fun(M, Fun, A) -> apply(M, Fun, A) end),
+ try Wrapper(Module, F, Args) of
Result ->
group_leader(OldGl, self()),
case lists:member(no_fail, Opts) of
--
2.35.3
