Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
home:Ledest:erlang:24
erlang
0514-kernel-Fix-os-cmd-error-report-for-open_po...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
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
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
