Merge branch 'bmk/snmp/improve_error_handling_reading_config/OTP-9943' into bmk/snmp/snmp422_integration/r15

Conflicts:
	lib/snmp/test/snmp_app_test.erl

6 files changed, 144 insertions, 38 deletions

diff --git a/lib/snmp/src/agent/snmp_standard_mib.erl b/lib/snmp/src/agent/snmp_standard_mib.erl
index 7c78e03b3b..ca93546923 100644
--- a/lib/snmp/src/agent/snmp_standard_mib.erl
+++ b/lib/snmp/src/agent/snmp_standard_mib.erl
@@ -156,7 +156,7 @@ read_standard(Dir) ->
     Gen        = fun(D, Reason) -> 
 			 throw({error, {failed_reading_config_file, 
 					D, FileName, 
-					file:list_dir(Dir), Reason}})
+					list_dir(Dir), Reason}})
 		 end,
     Filter     = fun(Standard) -> sort_standard(Standard) end,
     Check      = fun(Entry) -> check_standard(Entry) end,
@@ -164,6 +164,14 @@ read_standard(Dir) ->
 	snmp_conf:read_files(Dir, [{Gen, Filter, Check, FileName}]), 
     Standard.
 
+list_dir(Dir) ->
+    case file:list_dir(Dir) of
+	{ok, Files} ->
+	    Files;
+	Error ->
+	    Error
+    end.
+
 
 %%-----------------------------------------------------------------
 %% Make sure that each mandatory standard attribute is present, and
diff --git a/lib/snmp/src/app/snmp.appup.src b/lib/snmp/src/app/snmp.appup.src
index 8b0946d319..8360d88c94 100644
--- a/lib/snmp/src/app/snmp.appup.src
+++ b/lib/snmp/src/app/snmp.appup.src
@@ -24,6 +24,7 @@
  [
   {"4.21.7", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -49,6 +50,7 @@
   }, 
   {"4.21.6",
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -74,6 +76,7 @@
   },
   {"4.21.5",
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -104,6 +107,7 @@
   },
   {"4.21.4", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -137,6 +141,7 @@
   }, 
   {"4.21.3", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -170,6 +175,7 @@
   }, 
   {"4.21.2", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -205,6 +211,7 @@
   }, 
   {"4.21.1", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -241,6 +248,7 @@
   }, 
   {"4.21", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -282,6 +290,7 @@
  [
   {"4.21.7", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -307,6 +316,7 @@
   }, 
   {"4.21.6",
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -332,6 +342,7 @@
   },
   {"4.21.5",
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -362,6 +373,7 @@
   },
   {"4.21.4", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -395,6 +407,7 @@
   }, 
   {"4.21.3", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -428,6 +441,7 @@
   }, 
   {"4.21.2", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -463,6 +477,7 @@
   }, 
   {"4.21.1", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
@@ -499,6 +514,7 @@
   }, 
   {"4.21", 
    [
+    {load_module, snmp_config, soft_purge, soft_purge, []}, 
     {load_module, snmp_conf, soft_purge, soft_purge, []}, 
     {load_module, snmp_community_mib, soft_purge, soft_purge, [snmp_conf]}, 
     {load_module, snmp_framework_mib, soft_purge, soft_purge, [snmp_conf]}, 
diff --git a/lib/snmp/src/misc/snmp_config.erl b/lib/snmp/src/misc/snmp_config.erl
index 6ab20e3e48..1da2c339c7 100644
--- a/lib/snmp/src/misc/snmp_config.erl
+++ b/lib/snmp/src/misc/snmp_config.erl
@@ -90,6 +90,17 @@
 	]).
 
 
+-export_type([void/0, 
+	      verify_config_entry_function/0, 
+	      verify_config_function/0, 
+	      write_config_function/0]).
+
+
+%%----------------------------------------------------------------------
+
+-type void() :: term(). % Any value - ignored
+
+
 %%----------------------------------------------------------------------
 %% Handy SNMP configuration
 %%----------------------------------------------------------------------
@@ -2356,53 +2367,109 @@ write_sys_config_file_manager_atl_opt(Fid, {seqno, SeqNo}) ->
 
 
 header() ->
-    {Y,Mo,D} = date(),
-    {H,Mi,S} = time(),
+    {Y, Mo, D} = date(),
+    {H, Mi, S} = time(),
     io_lib:format("%% This file was generated by "
-		  "snmp_config (version-~s) ~w-~2.2.0w-~2.2.0w "
+		  "~w (version-~s) ~w-~2.2.0w-~2.2.0w "
 		  "~2.2.0w:~2.2.0w:~2.2.0w\n",
-		  [?version,Y,Mo,D,H,Mi,S]).
+		  [?MODULE, ?version, Y, Mo, D, H, Mi, S]).
+
 
+%% *If* these functions are successfull, they successfully return anything
+%% (which is ignored), but they fail with either a throw or an exit or 
+%% something similar.
+
+%% Verification of one config entry read from a file
+-type(verify_config_entry_function() :: 
+      fun((Entry :: term()) -> ok | {error, Reason :: term()})).
+
+%% Verification of config to be written
+-type(verify_config_function() :: 
+      fun(() -> void())).
+
+%% Write config to file (as defined by Fd)
+-type(write_config_function() :: 
+      fun((Fd :: file:io_device()) -> void())).
+
+-spec write_config_file(Dir :: string(), 
+			FileName :: string(), 
+			Verify :: verify_config_function(), 
+			Write :: write_config_function()) ->
+    ok | {error, Reason :: term()}.
 
 write_config_file(Dir, FileName, Verify, Write) 
   when (is_list(Dir) andalso 
 	is_list(FileName) andalso 
 	is_function(Verify) andalso 
 	is_function(Write)) ->
-    (catch do_write_config_file(Dir, FileName, Verify, Write)).
+    try
+	begin
+	    do_write_config_file(Dir, FileName, Verify, Write)
+	end
+    catch
+	throw:Error ->
+	    Error;
+	  T:E ->
+	    {error, {failed_write, Dir, FileName, T, E}}
+    end.
+
 
 do_write_config_file(Dir, FileName, Verify, Write) ->
     Verify(),
     case file:open(filename:join(Dir, FileName), [write]) of
 	{ok, Fd} ->
-	    (catch Write(Fd)),
-	    file:close(Fd),
-	    ok;
+	    file_write_and_close(Write, Fd, Dir, FileName);
 	Error ->
 	    Error
     end.
 
-
 append_config_file(Dir, FileName, Verify, Write) 
   when (is_list(Dir) andalso 
 	is_list(FileName) andalso 
 	is_function(Verify) andalso 
 	is_function(Write)) ->
-    (catch do_append_config_file(Dir, FileName, Verify, Write)).
+    try
+	begin
+	    do_append_config_file(Dir, FileName, Verify, Write)
+	end
+    catch
+	throw:Error ->
+	    Error;
+	  T:E ->
+	    {error, {failed_append, Dir, FileName, T, E}}
+    end.
 
 do_append_config_file(Dir, FileName, Verify, Write) ->
     Verify(),
     case file:open(filename:join(Dir, FileName), [read, write]) of
 	{ok, Fd} ->
 	    file:position(Fd, eof),
-	    (catch Write(Fd)),
-	    file:close(Fd),
-	    ok;
+	    file_write_and_close(Write, Fd, Dir, FileName);
 	Error ->
 	    Error
     end.
 
 
+file_write_and_close(Write, Fd, Dir, FileName) ->
+    ok = Write(Fd),
+    case file:sync(Fd) of
+	ok ->
+	    case file:close(Fd) of
+		ok ->
+		    ok;
+		{error, Reason} ->
+		    {error, {failed_closing, Dir, FileName, Reason}}
+	    end;
+	{error, Reason} ->
+	    {error, {failed_syncing, Dir, FileName, Reason}}
+    end.
+
+
+-spec read_config_file(Dir :: string(), 
+		       FileName :: string(), 
+		       Verify :: verify_config_entry_function()) ->
+    {ok, Config :: list()} | {error, Reason :: term()}.
+
 read_config_file(Dir, FileName, Verify) 
   when is_list(Dir) andalso is_list(FileName) andalso is_function(Verify) ->
     (catch do_read_config_file(Dir, FileName, Verify)).
diff --git a/lib/snmp/test/snmp_agent_test.erl b/lib/snmp/test/snmp_agent_test.erl
index b5acf7cd87..8ee5dd534d 100644
--- a/lib/snmp/test/snmp_agent_test.erl
+++ b/lib/snmp/test/snmp_agent_test.erl
@@ -224,8 +224,7 @@ groups() ->
      },
      {tickets2, [], [otp8395, otp9884]},
      {otp_4394, [], [otp_4394_test]},
-     {otp_7157, [], [otp_7157_test]
-     }
+     {otp_7157, [], [otp_7157_test]}
     ].
 
 
@@ -621,13 +620,13 @@ init_mib_storage_ets(Config) when is_list(Config) ->
 
 init_mib_storage_dets(Config) when is_list(Config) ->
     ?LOG("init_mib_storage_ets -> entry", []),
-    ?line AgentDir = ?GCONF(agent_dir, Config),
-    MibStorage = {snmp_mib_storage,{dets,AgentDir}},
+    ?line AgentDbDir = ?GCONF(agent_db_dir, Config),
+    MibStorage = {snmp_mib_storage, {dets, AgentDbDir}},
     init_ms(Config, [MibStorage]).
 
 init_mib_storage_mnesia(Config) when is_list(Config) ->
     ?LOG("init_mib_storage_ets -> entry", []),
-    MibStorage = {snmp_mib_storage,{mnesia,[]}},
+    MibStorage = {snmp_mib_storage, {mnesia,[]}},
     init_ms(Config, [MibStorage]).
 
 init_ms(Config, Opts) when is_list(Config) ->
@@ -650,8 +649,8 @@ init_size_check_mse(Config) when is_list(Config) ->
     init_size_check_ms(Config, [MibStorage]).
 
 init_size_check_msd(Config) when is_list(Config) ->
-    AgentDir   = ?GCONF(agent_dir, Config),
-    MibStorage = {snmp_mib_storage, {dets, AgentDir}},
+    AgentDbDir = ?GCONF(agent_db_dir, Config),
+    MibStorage = {snmp_mib_storage, {dets, AgentDbDir}},
     init_size_check_ms(Config, [MibStorage]).
 
 init_size_check_msm(Config) when is_list(Config) ->
@@ -673,22 +672,24 @@ init_size_check_ms(Config, Opts) when is_list(Config) ->
 	    ?SKIP({failed_starting_crypto, Reason})
     end,
     create_tables(SaNode),
-    AgentDir = ?GCONF(agent_dir, Config),
-    MgrDir = ?GCONF(mgr_dir, Config),
-    Ip = ?GCONF(ip, Config),
-    ?line ok = 
-	config([v3], MgrDir, AgentDir, tuple_to_list(Ip), tuple_to_list(Ip)),
+    AgentConfDir = ?GCONF(agent_conf_dir, Config),
+    MgrDir       = ?GCONF(mgr_dir, Config),
+    Ip           = ?GCONF(ip, Config),
+    ?line ok = config([v3], MgrDir, AgentConfDir, 
+		      tuple_to_list(Ip), tuple_to_list(Ip)),
     [{vsn, v3} | start_v3_agent(Config, Opts)].
 
 init_varm_mib_storage_dets(Config) when is_list(Config) ->
     ?LOG("init_varm_mib_storage_dets -> entry", []),
-    ?line SaNode   = ?GCONF(snmp_sa, Config),
+    ?line SaNode       = ?GCONF(snmp_sa, Config),
     ?line create_tables(SaNode),
-    ?line AgentDir = ?GCONF(agent_dir, Config),
-    ?line MgrDir   = ?GCONF(mgr_dir, Config),
-    ?line Ip       = ?GCONF(ip, Config),
-    ?line config([v1], MgrDir, AgentDir, tuple_to_list(Ip), tuple_to_list(Ip)),
-    MibStorage           = {snmp_mib_storage,{dets,AgentDir}},
+    ?line AgentDbDir   = ?GCONF(agent_db_dir, Config),
+    ?line AgentConfDir = ?GCONF(agent_conf_dir, Config),
+    ?line MgrDir       = ?GCONF(mgr_dir, Config),
+    ?line Ip           = ?GCONF(ip, Config),
+    ?line config([v1], MgrDir, AgentConfDir, 
+		 tuple_to_list(Ip), tuple_to_list(Ip)),
+    MibStorage           = {snmp_mib_storage, {dets, AgentDbDir}},
     MasterAgentVerbosity = {snmp_master_agent_verbosity,   trace},
     MibsVerbosity        = {snmp_mibserver_verbosity,      trace},
     SymStoreVerbosity    = {snmp_symbolic_store_verbosity, trace},
@@ -697,12 +698,13 @@ init_varm_mib_storage_dets(Config) when is_list(Config) ->
 
 init_varm_mib_storage_mnesia(Config) when is_list(Config) ->
     ?LOG("init_varm_mib_storage_mnesia -> entry", []),
-    ?line SaNode   = ?GCONF(snmp_sa, Config),
+    ?line SaNode       = ?GCONF(snmp_sa, Config),
     ?line create_tables(SaNode),
-    ?line AgentDir = ?GCONF(agent_dir, Config),
-    ?line MgrDir   = ?GCONF(mgr_dir, Config),
-    ?line Ip       = ?GCONF(ip, Config),
-    ?line config([v1], MgrDir, AgentDir, tuple_to_list(Ip), tuple_to_list(Ip)),
+    ?line AgentConfDir = ?GCONF(agent_conf_dir, Config),
+    ?line MgrDir       = ?GCONF(mgr_dir, Config),
+    ?line Ip           = ?GCONF(ip, Config),
+    ?line config([v1], MgrDir, AgentConfDir, 
+		 tuple_to_list(Ip), tuple_to_list(Ip)),
     MibStorage           = {snmp_mib_storage,{mnesia,[]}},
     MasterAgentVerbosity = {snmp_master_agent_verbosity,   trace},
     MibsVerbosity        = {snmp_mibserver_verbosity,      trace},
diff --git a/lib/snmp/test/snmp_app_test.erl b/lib/snmp/test/snmp_app_test.erl
index 825a232c99..0a320dbbce 100644
--- a/lib/snmp/test/snmp_app_test.erl
+++ b/lib/snmp/test/snmp_app_test.erl
@@ -23,7 +23,8 @@
 -module(snmp_app_test).
 
 -export([
-	 all/0, groups/0, init_per_group/2, end_per_group/2, 
+	 all/0, groups/0, 
+	 init_per_group/2, end_per_group/2, 
 	 init_per_suite/1, end_per_suite/1,
 	 init_per_testcase/2, end_per_testcase/2, 
 
diff --git a/lib/snmp/test/snmp_test_lib.erl b/lib/snmp/test/snmp_test_lib.erl
index bc54361aaf..26115a0c74 100644
--- a/lib/snmp/test/snmp_test_lib.erl
+++ b/lib/snmp/test/snmp_test_lib.erl
@@ -211,6 +211,10 @@ fix_data_dir(Config) ->
 
 
 init_suite_top_dir(Suite, Config0) ->
+    io:format("~w:init_suite_top_dir -> entry with"
+	      "~n   Suite:   ~p"
+	      "~n   Config0: ~p"
+	      "~n", [?MODULE, Suite, Config0]),
     Dir         = lookup(priv_dir, Config0),
     SuiteTopDir = filename:join(Dir, Suite),
     case file:make_dir(SuiteTopDir) of
@@ -230,6 +234,10 @@ init_suite_top_dir(Suite, Config0) ->
 
 
 init_group_top_dir(GroupName, Config) ->
+    io:format("~w:init_group_top_dir -> entry with"
+	      "~n   GroupName: ~p"
+	      "~n   Config:    ~p"
+	      "~n", [?MODULE, GroupName, Config]),
     case lists:keysearch(snmp_group_top_dir, 1, Config) of
 	{value, {_Key, Dir}} ->
 	    %% This is a sub-group, so create our dir within Dir
@@ -263,6 +271,10 @@ init_group_top_dir(GroupName, Config) ->
 
 
 init_testcase_top_dir(Case, Config) ->
+    io:format("~w:init_testcase_top_dir -> entry with"
+	      "~n   Case:   ~p"
+	      "~n   Config: ~p"
+	      "~n", [?MODULE, Case, Config]),
     case lists:keysearch(snmp_group_top_dir, 1, Config) of
 	{value, {_Key, Dir}} ->
 	    CaseTopDir = filename:join(Dir, Case),