aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSiri Hansen <[email protected]>2019-03-05 11:25:00 +0100
committerSiri Hansen <[email protected]>2019-03-06 12:10:26 +0100
commite03c2bd640e1fc1503e92dfd74991f4205973c61 (patch)
tree86dd52b45e65b7a1fae4b27947ae5bc632ebefb6
parent0fb9b241d11588bdae29aa64ff96d1eb67d230af (diff)
downloadotp-e03c2bd640e1fc1503e92dfd74991f4205973c61.tar.gz
otp-e03c2bd640e1fc1503e92dfd74991f4205973c61.tar.bz2
otp-e03c2bd640e1fc1503e92dfd74991f4205973c61.zip
[logger] Add better control of file modes in logger_std_h
OTP-15602 It is allowed to set file modes for the handler to use when opening its log file. The given modes were earlier accepted without any checks, which could make the handler behave unexpectedly. This commit makes sure that * if none of write, append or exclusive is given, then append is added * if raw is not given, it is added * if delayed_write or {delayed_write,_,_} is not given, then delayed_write is added
-rw-r--r--lib/kernel/doc/src/logger_std_h.xml23
-rw-r--r--lib/kernel/src/logger_std_h.erl50
-rw-r--r--lib/kernel/test/logger_SUITE.erl11
-rw-r--r--lib/kernel/test/logger_std_h_SUITE.erl23
4 files changed, 72 insertions, 35 deletions
diff --git a/lib/kernel/doc/src/logger_std_h.xml b/lib/kernel/doc/src/logger_std_h.xml
index fcd180abd6..8c5a476d86 100644
--- a/lib/kernel/doc/src/logger_std_h.xml
+++ b/lib/kernel/doc/src/logger_std_h.xml
@@ -59,14 +59,21 @@
<item>
<p>This has the value <c>standard_io</c>, <c>standard_error</c>,
<c>{file,LogFileName}</c>, or <c>{file,LogFileName,LogFileOpts}</c>.</p>
- <p>If <c>LogFileOpts</c> is specified, it replaces the default
- list of options used when opening the log file. The default
- list is <c>[raw,append,delayed_write]</c>. One reason to do
- so can be to change <c>append</c> to, for
- example, <c>write</c>, ensuring that the old log is
- truncated when a node is restarted. See the reference manual
- for <seealso marker="file#open-2"><c>file:open/2</c></seealso>
- for more information about file options.</p>
+ <p><c>LogFileOpts</c> specify the file options used when
+ opening the log file,
+ see <seealso marker="file#open-2"><c>file:open/2</c></seealso>.
+ If <c>LogFileOpts</c> is not specified, the default option
+ list used is <c>[raw,append,delayed_write]</c>. If <c>LogFileOpts</c>
+ is specified, it replaces the default options list with the
+ following adjustments:</p>
+ <list>
+ <item>
+ If <c>raw</c> is not found in the list, it is added
+ </item>
+ <item>
+ It <c>write</c>, <c>append</c> or <c>exclusice</c> are not
+ found in the list, <c>append</c> is added</item>
+ </list>
<p>Log files are always UTF-8 encoded. The encoding can not be
changed by setting the option <c>{encoding,Encoding}</c>
in <c>LogFileOpts</c>.</p>
diff --git a/lib/kernel/src/logger_std_h.erl b/lib/kernel/src/logger_std_h.erl
index 392ac7e67b..c634f86550 100644
--- a/lib/kernel/src/logger_std_h.erl
+++ b/lib/kernel/src/logger_std_h.erl
@@ -136,12 +136,10 @@ check_config(_Name,SetOrUpdate,OldHConfig,NewHConfig0) ->
{error,{illegal_config_change,?MODULE,WriteOnce,Other}}
end.
-check_config(#{type:=Type}=HConfig) ->
+check_config(HConfig) ->
case check_h_config(maps:to_list(HConfig)) of
- ok when is_atom(Type) ->
- {ok,HConfig#{filesync_repeat_interval=>no_repeat}};
ok ->
- {ok,HConfig};
+ {ok,fix_file_opts(HConfig)};
{error,{Key,Value}} ->
{error,{invalid_config,?MODULE,#{Key=>Value}}}
end.
@@ -162,8 +160,40 @@ check_h_config([]) ->
get_default_config() ->
#{type => standard_io}.
-filesync(_Name, _Mode, #{type := Type}=State) when is_atom(Type) ->
- {ok,State};
+fix_file_opts(#{type:={file,File}}=HConfig) ->
+ fix_file_opts(HConfig#{type=>{file,File,[raw,append,delayed_write]}});
+fix_file_opts(#{type:={file,File,[]}}=HConfig) ->
+ fix_file_opts(HConfig#{type=>{file,File,[raw,append,delayed_write]}});
+fix_file_opts(#{type:={file,File,Modes}}=HConfig) ->
+ HConfig#{type=>{file,File,fix_modes(Modes)}};
+fix_file_opts(HConfig) ->
+ HConfig#{filesync_repeat_interval=>no_repeat}.
+
+fix_modes(Modes) ->
+ %% Ensure write|append|exclusive
+ Modes1 =
+ case [M || M <- Modes,
+ lists:member(M,[write,append,exclusive])] of
+ [] -> [append|Modes];
+ _ -> Modes
+ end,
+ %% Ensure raw
+ Modes2 =
+ case lists:member(raw,Modes) of
+ false -> [raw|Modes1];
+ true -> Modes1
+ end,
+ %% Ensure delayed_write
+ case lists:partition(fun(delayed_write) -> true;
+ ({delayed_write,_,_}) -> true;
+ (_) -> false
+ end, Modes2) of
+ {[],_} ->
+ [delayed_write|Modes2];
+ _ ->
+ Modes2
+ end.
+
filesync(_Name, async, #{file_ctrl_pid := FileCtrlPid} = State) ->
ok = file_ctrl_filesync_async(FileCtrlPid),
{ok,State};
@@ -217,12 +247,6 @@ open_log_file(HandlerName, FileInfo) ->
Error -> Error
end.
-do_open_log_file({file,FileName}) ->
- do_open_log_file({file,FileName,[raw,append,delayed_write]});
-
-do_open_log_file({file,FileName,[]}) ->
- do_open_log_file({file,FileName,[raw,append,delayed_write]});
-
do_open_log_file({file,FileName,Modes}) ->
try
case filelib:ensure_dir(FileName) of
@@ -323,7 +347,7 @@ file_ctrl_init(HandlerName, StdDev, Starter) ->
%% Modify file options to use when re-opening if the inode has
%% changed. I.e. the file may exist and if so should be appended to.
set_file_opt_append({file, FileName, Modes}) ->
- {file, FileName, [append | Modes--[exclusive]]};
+ {file, FileName, [append | Modes--[write,append,exclusive]]};
set_file_opt_append(FileInfo) ->
FileInfo.
diff --git a/lib/kernel/test/logger_SUITE.erl b/lib/kernel/test/logger_SUITE.erl
index 2dad651f9c..ce0c2efa10 100644
--- a/lib/kernel/test/logger_SUITE.erl
+++ b/lib/kernel/test/logger_SUITE.erl
@@ -1047,8 +1047,9 @@ kernel_config(Config) ->
ok = rpc:call(Node,application,unset_env,[kernel,logger]),
ok = rpc:call(Node,logger,internal_init_logger,[]),
ok = rpc:call(Node,logger,add_handlers,[kernel]),
+ DefModes = [raw,append,delayed_write],
#{primary:=#{filter_default:=log,filters:=[]},
- handlers:=[#{id:=default,filters:=DF,config:=#{type:={file,F}}}],
+ handlers:=[#{id:=default,filters:=DF,config:=#{type:={file,F,DefModes}}}],
module_levels:=[]} = rpc:call(Node,logger,get_config,[]),
%% Same, but using 'logger' parameter instead of 'error_logger'
@@ -1060,26 +1061,26 @@ kernel_config(Config) ->
ok = rpc:call(Node,logger,internal_init_logger,[]),
ok = rpc:call(Node,logger,add_handlers,[kernel]),
#{primary:=#{filter_default:=log,filters:=[]},
- handlers:=[#{id:=default,filters:=DF,config:=#{type:={file,F}}}],
+ handlers:=[#{id:=default,filters:=DF,config:=#{type:={file,F,DefModes}}}],
module_levels:=[]} = rpc:call(Node,logger,get_config,[]),
%% Same, but with type={file,File,Modes}
ok = rpc:call(Node,logger,remove_handler,[default]),% so it can be added again
ok = rpc:call(Node,application,unset_env,[kernel,error_logger]),
- M = [raw,write,delayed_write],
+ M = [raw,write],
ok = rpc:call(Node,application,set_env,[kernel,logger,
[{handler,default,logger_std_h,
#{config=>#{type=>{file,F,M}}}}]]),
ok = rpc:call(Node,logger,internal_init_logger,[]),
ok = rpc:call(Node,logger,add_handlers,[kernel]),
#{primary:=#{filter_default:=log,filters:=[]},
- handlers:=[#{id:=default,filters:=DF,config:=#{type:={file,F,M}}}],
+ handlers:=[#{id:=default,filters:=DF,
+ config:=#{type:={file,F,[delayed_write|M]}}}],
module_levels:=[]} = rpc:call(Node,logger,get_config,[]),
%% Same, but with disk_log handler
ok = rpc:call(Node,logger,remove_handler,[default]),% so it can be added again
ok = rpc:call(Node,application,unset_env,[kernel,error_logger]),
- M = [raw,write,delayed_write],
ok = rpc:call(Node,application,set_env,[kernel,logger,
[{handler,default,logger_disk_log_h,
#{config=>#{file=>F}}}]]),
diff --git a/lib/kernel/test/logger_std_h_SUITE.erl b/lib/kernel/test/logger_std_h_SUITE.erl
index ba491ac5fa..8cadb1083b 100644
--- a/lib/kernel/test/logger_std_h_SUITE.erl
+++ b/lib/kernel/test/logger_std_h_SUITE.erl
@@ -257,9 +257,10 @@ errors(Config) ->
end,
{error,
- {handler_not_added,{open_failed,Log,_}}} =
+ {handler_not_added,
+ {invalid_config,logger_std_h,#{type:={file,Log,bad_file_opt}}}}} =
logger:add_handler(myh3,logger_std_h,
- #{config=>#{type=>{file,Log,[bad_file_opt]}}}),
+ #{config=>#{type=>{file,Log,bad_file_opt}}}),
ok = logger:notice(?msg).
@@ -607,11 +608,14 @@ reconfig(cleanup, _Config) ->
file_opts(Config) ->
Dir = ?config(priv_dir,Config),
Log = filename:join(Dir, lists:concat([?FUNCTION_NAME,".log"])),
- BadFileOpts = [raw],
- BadType = {file,Log,BadFileOpts},
- {error,{handler_not_added,{open_failed,Log,enoent}}} =
- logger:add_handler(?MODULE, logger_std_h,
- #{config => #{type => BadType}}),
+ MissingAccess = [raw],
+ Type1 = {file,Log,MissingAccess},
+ ok = logger:add_handler(?MODULE, logger_std_h,
+ #{config => #{type => Type1}}),
+ {ok,#{config:=#{type:={file,Log,Modes1}}}} =
+ logger:get_handler_config(?MODULE),
+ [append,delayed_write,raw] = lists:sort(Modes1),
+ ok = logger:remove_handler(?MODULE),
OkFileOpts = [raw,append],
OkType = {file,Log,OkFileOpts},
@@ -622,9 +626,10 @@ file_opts(Config) ->
filters=>?DEFAULT_HANDLER_FILTERS([?MODULE]),
formatter=>{?MODULE,self()}}),
- #{cb_state := #{handler_state := #{type := OkType}}} =
+ ModType = {file,Log,[delayed_write|OkFileOpts]},
+ #{cb_state := #{handler_state := #{type := ModType}}} =
logger_olp:info(h_proc_name()),
- {ok,#{config := #{type := OkType}}} = logger:get_handler_config(?MODULE),
+ {ok,#{config := #{type := ModType}}} = logger:get_handler_config(?MODULE),
logger:notice(M1=?msg,?domain),
?check(M1),
B1 = ?bin(M1),