Merge branch 'bmk/snmp/agent/vacmAccessTable-RowStatus-handling/OTP-10164' into bmk/snmp/snmp4221_integration

Conflicts:
	lib/snmp/doc/src/notes.xml
	lib/snmp/src/app/snmp.appup.src

3 files changed, 92 insertions, 41 deletions

diff --git a/lib/snmp/doc/src/notes.xml b/lib/snmp/doc/src/notes.xml
index 048623c61e..ea5648b808 100644
--- a/lib/snmp/doc/src/notes.xml
+++ b/lib/snmp/doc/src/notes.xml
@@ -41,18 +41,27 @@
 
     <section>
       <title>Improvements and new features</title>
+<!--
       <p>-</p>
+-->
 
-<!--
       <list type="bulleted">
         <item>
-          <p>[agent] Improve error handling while reading agent config files. 
-	  Some files contain mandatory information and is therefor themself
-	  mandatory. </p>
-	  <p>Own Id: OTP-9943</p>
+          <p>[agent] Errors in vacmAccessTable RowStatus handling.
+	  There are problems with the handling of vacmAccessTableStatus 
+	  that cause some SNMP test suites to report errors.
+	  Most notably, erroneous set operations frequently cause "genErr" 
+	  errors to be returned. These "genErr" errors are usually caused 
+	  by badmatch exceptions coming from 
+	  <c>{ok, Row} = snmpa_vacm:get_row(RowIndex)</c> 
+	  if the row does not exist. </p>
+	  <p>The semantics of the RowStatus handling in that table has 
+	  been adjusted to be compliant with the RowStatus
+	  textual description of SNPMv2-TC MIB. </p>
+	  <p>Stefan Zegenhagen</p>
+	  <p>Own Id: OTP-10164</p>
         </item>
       </list>
--->
 
     </section>
 
diff --git a/lib/snmp/src/agent/snmp_view_based_acm_mib.erl b/lib/snmp/src/agent/snmp_view_based_acm_mib.erl
index 479a44795f..e7dea52857 100644
--- a/lib/snmp/src/agent/snmp_view_based_acm_mib.erl
+++ b/lib/snmp/src/agent/snmp_view_based_acm_mib.erl
@@ -565,45 +565,85 @@ vacmAccessTable(is_set_ok, RowIndex, Cols0) ->
     case (catch verify_vacmAccessTable_cols(Cols0, [])) of
 	{ok, Cols} ->
 	    IsValidKey = is_valid_key(RowIndex),
-	    case lists:keysearch(?vacmAccessStatus, 1, Cols) of
-		%% Ok, if contextMatch is init
-		{value, {Col, ?'RowStatus_active'}} -> 
-	            {ok, Row} = snmpa_vacm:get_row(RowIndex),
-	            case element(?vacmAContextMatch, Row) of
-			noinit -> {inconsistentValue, Col};
-			_ -> {noError, 0}
+	    StatusCol  = lists:keyfind(?vacmAccessStatus, 1, Cols),
+	    MaybeRow   = snmpa_vacm:get_row(RowIndex),
+	    case {StatusCol, MaybeRow} of
+		{{Col, ?'RowStatus_active'}, false} ->
+		    %% row does not yet exist => inconsistentValue
+		    %% (see SNMPv2-TC.mib, RowStatus textual convention)
+		    {inconsistentValue, Col};
+		{{Col, ?'RowStatus_active'}, {ok, Row}} ->
+		    %% Ok, if contextMatch is init
+		    case element(?vacmAContextMatch, Row) of
+			noinit ->
+			    %% check whether ContextMatch is being set in
+			    %% the same operation
+			    case proplists:get_value(?vacmAccessContextMatch, 
+						     Cols) of
+				undefined ->
+				    %% no, we can't make this row active yet
+				    {inconsistentValue, Col};
+				_ ->
+				    %% ok, activate the row
+				    {noError, 0}
+			    end;
+			_ ->
+			    {noError, 0}
 		    end;
-		{value, {Col, ?'RowStatus_notInService'}} -> % Ok, if not notReady
-		    {ok, Row} = snmpa_vacm:get_row(RowIndex),
+		{{Col, ?'RowStatus_notInService'}, false} ->
+		    %% row does not yet exist => inconsistentValue
+		    %% (see SNMPv2-TC.mib, RowStatus textual convention)
+		    {inconsistentValue, Col};
+		{{Col, ?'RowStatus_notInService'}, {ok, Row}} ->
+		    %% Ok, if not notReady
 		    case element(?vacmAStatus, Row) of
-			?'RowStatus_notReady' -> {inconsistentValue, Col};
-			_ -> {noError, 0}
+			?'RowStatus_notReady' ->
+			    {inconsistentValue, Col};
+			_ ->
+			    {noError, 0}
 		    end;
-		{value, {Col, ?'RowStatus_notReady'}} -> % never ok!
+		{{Col, ?'RowStatus_notReady'}, _} ->
+		    %% never ok!
 		    {inconsistentValue, Col};
-		{value, {Col, ?'RowStatus_createAndGo'}} -> % ok, if it doesn't exist
+		{{Col, ?'RowStatus_createAndGo'}, false} ->
+		    %% ok, if it doesn't exist
 		    Res = lists:keysearch(?vacmAccessContextMatch, 1, Cols),
-		    case snmpa_vacm:get_row(RowIndex) of
-			false when (IsValidKey =:= true) andalso 
-				   is_tuple(Res) -> {noError, 0};
-			false -> {noCreation, Col}; % Bad RowIndex
-			_ -> {inconsistentValue, Col}
+		    if
+			IsValidKey =/= true ->
+			    %% bad RowIndex => noCreation
+			    {noCreation, Col};
+			is_tuple(Res) ->
+			    %% required field is present => noError
+			    {noError, 0};
+			true ->
+			    %% required field is missing => inconsistentValue
+			    {inconsistentValue, Col}
 		    end;
-		{value, {Col, ?'RowStatus_createAndWait'}} -> % ok, if it doesn't exist
-		    case snmpa_vacm:get_row(RowIndex) of
-			false when (IsValidKey =:= true) -> {noError, 0};
-			false -> {noCreation, Col}; % Bad RowIndex
-			_ -> {inconsistentValue, Col}
+		{{Col, ?'RowStatus_createAndGo'}, _} ->
+		    %% row already exists => inconsistentValue
+		    {inconsistentValue, Col};
+		{{Col, ?'RowStatus_createAndWait'}, false} ->
+		    %% ok, if it doesn't exist
+		    if
+			IsValidKey =:= true ->
+			    %% RowIndex is valid => noError
+			    {noError, 0};
+			true ->
+			    {noCreation, Col}
 		    end;
-		{value, {_Col, ?'RowStatus_destroy'}} -> % always ok!
+		{{Col, ?'RowStatus_createAndWait'}, _} ->
+		    %% Row already exists => inconsistentValue
+		    {inconsistentValue, Col};
+		{value, {_Col, ?'RowStatus_destroy'}} ->
+		    %% always ok!
 		    {noError, 0};
-		_ -> % otherwise, it's a change; it must exist
-		    case snmpa_vacm:get_row(RowIndex) of
-			{ok, _} ->
-			    {noError, 0};
-			false ->
-			    {inconsistentName, element(1, hd(Cols))}
-		    end
+		{_, false} ->
+		    %% otherwise, it's a row change; 
+		    %% row does not exist => inconsistentName
+		    {inconsistentName, element(1, hd(Cols))};
+		_ ->
+		    %% row change and row exists => noError
+		    {noError, 0}
 	    end;
 	Error ->
 	    Error
diff --git a/lib/snmp/src/app/snmp.appup.src b/lib/snmp/src/app/snmp.appup.src
index 91dfc50dc9..3991cdbce8 100644
--- a/lib/snmp/src/app/snmp.appup.src
+++ b/lib/snmp/src/app/snmp.appup.src
@@ -24,8 +24,9 @@
  [
   {"4.22", 
    [
-    {load_module, snmpm,     soft_purge, soft_purge, []}, 
-    {load_module, snmp_pdus, soft_purge, soft_purge, []}
+    {load_module, snmpm,                   soft_purge, soft_purge, []}, 
+    {load_module, snmp_pdus,               soft_purge, soft_purge, []}, 
+    {load_module, snmp_view_based_acm_mib, soft_purge, soft_purge, []}
    ]
   }, 
   {"4.21.7", 
@@ -312,8 +313,9 @@
  [
   {"4.22", 
    [
-    {load_module, snmpm,     soft_purge, soft_purge, []}, 
-    {load_module, snmp_pdus, soft_purge, soft_purge, []}
+    {load_module, snmpm,                   soft_purge, soft_purge, []}, 
+    {load_module, snmp_pdus,               soft_purge, soft_purge, []}, 
+    {load_module, snmp_view_based_acm_mib, soft_purge, soft_purge, []}
    ]
   }, 
   {"4.21.7",