Check INTEGER values better

There is duplicated effort in that validate_integer() checks
whether the integer value is valid, and then normalize_integer()
does mostly the same work in order to convert the value to an
integer.

Eliminate the validate_integer() function and incorporate
its checks into normalize_integer(). Also produce proper
error messages.

4 files changed, 58 insertions, 69 deletions

diff --git a/lib/asn1/src/asn1ct_check.erl b/lib/asn1/src/asn1ct_check.erl
index f1437597ec..3f32a77c1d 100644
--- a/lib/asn1/src/asn1ct_check.erl
+++ b/lib/asn1/src/asn1ct_check.erl
@@ -2123,7 +2123,6 @@ check_valuedef(#state{recordtopname=TopName}=S0, V0) ->
     #valuedef{name=Name,type=Vtype,value=Value,module=ModName} = V0,
     V = V0#valuedef{checked=true},
     Def = Vtype#type.def,
-    Constr = Vtype#type.constraint,
     S1 = S0#state{type=Vtype,tname=Def,value=V0,vname=Name},
     SVal = update_state(S1, ModName),
     case Def of
@@ -2159,10 +2158,8 @@ check_valuedef(#state{recordtopname=TopName}=S0, V0) ->
 						  module=ModName}),
 	    V#valuedef{value=CheckedV#valuedef.value};
 	'INTEGER' ->
-	    ok = validate_integer(SVal, Value, [], Constr),
 	    V#valuedef{value=normalize_value(SVal, Vtype, Value, [])};
-	{'INTEGER',NamedNumberList} ->
-	    ok = validate_integer(SVal, Value, NamedNumberList, Constr),
+	{'INTEGER',_NamedNumberList} ->
 	    V#valuedef{value=normalize_value(SVal, Vtype, Value, [])};
 	#'SEQUENCE'{} ->
 	    {ok,SeqVal} = convert_external(SVal, Value),
@@ -2185,50 +2182,6 @@ is_contextswitchtype(#typedef{name='CHARACTER STRING'}) ->
 is_contextswitchtype(_) ->
     false.
 
-% validate_integer(S,{identifier,Pos,Id},NamedNumberList,Constr) ->
-%     case lists:keysearch(Id,1,NamedNumberList) of
-% 	{value,_} -> ok;
-% 	false -> error({value,"unknown NamedNumber",S})
-%     end;
-%% This case occurs when there is a valuereference
-%% validate_integer(S=#state{mname=M},
-%% 		 #'Externalvaluereference'{module=M,value=Id}=Ref,
-validate_integer(S,#'Externalvaluereference'{value=Id}=Ref,
-		 NamedNumberList,Constr) ->
-    case lists:keysearch(Id,1,NamedNumberList) of
-	{value,_} -> ok;
-	false -> validate_integer_ref(S,Ref,NamedNumberList,Constr)
-	    %%error({value,"unknown NamedNumber",S})
-    end;
-validate_integer(S,Id,NamedNumberList,Constr) when is_atom(Id) ->
-    case lists:keysearch(Id,1,NamedNumberList) of
-	{value,_} -> ok;
-	false -> validate_integer_ref(S,Id,NamedNumberList,Constr)
-		     %error({value,"unknown NamedNumber",S})
-    end;
-validate_integer(_S,Value,_NamedNumberList,Constr) when is_integer(Value) ->
-    check_integer_range(Value,Constr).
-
-validate_integer_ref(S,Id,_,_) when is_atom(Id) ->
-    error({value,"unknown integer referens",S});
-validate_integer_ref(S,Ref,NamedNumberList,Constr) ->
-    case get_referenced_type(S,Ref) of
-	{M,V} when is_record(V,valuedef) -> 
-	    NewS = update_state(S,M),
-	    case check_value(NewS,V) of
-		#valuedef{type=#type{def='INTEGER'},value=Value} ->
-		    validate_integer(NewS,Value,NamedNumberList,Constr);
-		_Err -> error({value,"unknown integer referens",S})
-	    end;
-	_ ->
-	    error({value,"unknown integer referens",S})
-    end.
-	    
-		
-
-check_integer_range(_Int, Constr) when is_list(Constr) ->
-    ok.
-
 %%------------
 %% This can be removed when the old parser is removed
 %% The function removes 'space' atoms from the list
@@ -2475,7 +2428,7 @@ normalize_value(S0, Type, {'DEFAULT',Value}, NameList) ->
 	{'BOOLEAN',CType,_} ->
 	    normalize_boolean(S,Value,CType);
 	{'INTEGER',CType,_} ->
-	    normalize_integer(S,Value,CType);
+	    normalize_integer(S0, Value, CType);
 	{'BIT STRING',CType,_} ->
 	    normalize_bitstring(S,Value,CType);
 	{'OCTET STRING',CType,_} ->
@@ -2526,28 +2479,25 @@ normalize_boolean(S,Bool=#'Externalvaluereference'{},CType) ->
 normalize_boolean(_,Other,_) ->
     throw({error,{asn1,{'invalid default value',Other}}}).
 
-normalize_integer(_S,Int,_) when is_integer(Int) ->
+normalize_integer(_S, Int, _) when is_integer(Int) ->
     Int;
-normalize_integer(_S,{Name,Int},_) when is_atom(Name),is_integer(Int) ->
-    Int;
-normalize_integer(S,{Name,Int=#'Externalvaluereference'{}},
-		  Type) when is_atom(Name) ->
-    normalize_integer(S,Int,Type);
-normalize_integer(S,Int=#'Externalvaluereference'{value=Name},Type) ->
-    case Type of
-	NNL when is_list(NNL) ->
-	    case lists:keysearch(Name,1,NNL) of
-		{value,{Name,Val}} ->
+normalize_integer(S, #'Externalvaluereference'{value=Name}=Ref, NNL) ->
+    case lists:keyfind(Name, 1, NNL) of
+	{Name,Val} ->
+	    Val;
+	false ->
+	    try get_referenced_value(S, Ref) of
+		Val when is_integer(Val) ->
 		    Val;
-		false ->
-		    get_normalized_value(S,Int,Type,
-					 fun normalize_integer/3,[])
-	    end;
-	_ ->
-	    get_normalized_value(S,Int,Type,fun normalize_integer/3,[])
+		_ ->
+		    asn1_error(S, S#state.value, illegal_integer_value)
+	    catch
+		throw:_ ->
+		    asn1_error(S, S#state.value, illegal_integer_value)
+	    end
     end;
-normalize_integer(_,Int,_) ->
-    exit({'Unknown INTEGER value',Int}).
+normalize_integer(#state{value=Val}=S, _, _) ->
+    asn1_error(S, Val, illegal_integer_value).
 
 %% normalize_bitstring(S, Value, Type) -> bitstring()
 %%  Convert a literal value for a BIT STRING to an Erlang bit string.
@@ -2926,6 +2876,8 @@ call_Func(S,Val,Type,Func,ArgList) ->
 get_canonic_type(S,Type,NameList) ->
     {InnerType,NewType,NewNameList} =
 	case Type#type.def of
+	    'INTEGER'=Name ->
+		{Name,[],NameList};
 	    Name when is_atom(Name) ->
 		{Name,Type,NameList};
 	    Ref when is_record(Ref,'Externaltypereference') ->
@@ -4526,6 +4478,14 @@ check_reference(S,#'Externaltypereference'{pos=Pos,module=Emod,type=Name}) ->
 	    #'Externaltypereference'{pos=Pos,module=ModName,type=Name}
     end.
 
+get_referenced_value(S, T) ->
+    case get_referenced_type(S, T) of
+	{ExtMod,#valuedef{value=#'Externalvaluereference'{}=Ref}} ->
+	    get_referenced_value(update_state(S, ExtMod), Ref);
+	{_,#valuedef{value=Val}} ->
+	    Val
+    end.
+
 get_referenced_type(S, T) ->
     case do_get_referenced_type(S, T) of
 	{_,#type{def=#'Externaltypereference'{}=ERef}} ->
@@ -6730,6 +6690,8 @@ format_error({illegal_instance_of,Class}) ->
     io_lib:format("using INSTANCE OF on class '~s' is illegal, "
 		  "because INSTANCE OF may only be used on the class TYPE-IDENTIFIER",
 		  [Class]);
+format_error(illegal_integer_value) ->
+    "expecting an integer value";
 format_error(illegal_octet_string_value) ->
     "expecting a bstring or an hstring as value for an OCTET STRING";
 format_error({illegal_typereference,Name}) ->
diff --git a/lib/asn1/test/asn1_SUITE_data/ValueTest.asn b/lib/asn1/test/asn1_SUITE_data/ValueTest.asn
index dae9ae498a..8db2fcf17d 100644
--- a/lib/asn1/test/asn1_SUITE_data/ValueTest.asn
+++ b/lib/asn1/test/asn1_SUITE_data/ValueTest.asn
@@ -50,4 +50,15 @@ my-universalstring UniversalString ::= {"This is a capital A: ",
 					greekCapitalLetterSigma,
 					"; try and spot the difference!"}
 
+-- Useful parameterized SEQUENCE.
+ParamSeq{Type} ::= SEQUENCE {
+    a Type
+}
+
+-- Integer values.
+IntegerSeq ::= ParamSeq{INTEGER}
+someInteger INTEGER ::= 42
+integerSeq1 IntegerSeq ::= { a otherInteger }
+otherInteger INTEGER ::= someInteger
+
 END
diff --git a/lib/asn1/test/error_SUITE.erl b/lib/asn1/test/error_SUITE.erl
index 713f80cbee..36ec7a8203 100644
--- a/lib/asn1/test/error_SUITE.erl
+++ b/lib/asn1/test/error_SUITE.erl
@@ -255,6 +255,11 @@ values(Config) ->
 	   "  os1 OCTET STRING ::= \"abc\"\n"
 	   "  os2 OCTET STRING ::= 42\n"
 	   "  os3 OCTET STRING ::= { 1, 3 }\n"
+	   "  os4 OCTET STRING ::= '1234'H\n"
+
+	   "  int1 INTEGER ::= \"string\"\n"
+	   "  int2 INTEGER ::= os4\n"
+	   "  int3 INTEGER ::= not-defined\n"
 	   "END\n">>},
     {error,
      [
@@ -263,7 +268,13 @@ values(Config) ->
       {structured_error,{M,3},asn1ct_check,
        illegal_octet_string_value},
       {structured_error,{M,4},asn1ct_check,
-       illegal_octet_string_value}
+       illegal_octet_string_value},
+      {structured_error,{M,6},asn1ct_check,
+       illegal_integer_value},
+      {structured_error,{M,7},asn1ct_check,
+       illegal_integer_value},
+      {structured_error,{M,8},asn1ct_check,
+       illegal_integer_value}
      ]
     } = run(P, Config),
     ok.
diff --git a/lib/asn1/test/testValueTest.erl b/lib/asn1/test/testValueTest.erl
index 8359930256..fdd8b006e2 100644
--- a/lib/asn1/test/testValueTest.erl
+++ b/lib/asn1/test/testValueTest.erl
@@ -58,4 +58,9 @@ main() ->
      [0,0,3,145],
      "; try and spot the difference!"] = M:'my-universalstring'(),
 
+    %% Integers
+    42 = M:someInteger(),
+    42 = M:otherInteger(),
+    {'IntegerSeq',42} = M:integerSeq1(),
+
     ok.