From 943266c923535a22050c18211d9f4ef95919aebc Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 1 Dec 2011 14:29:39 +0100 Subject: Vendor id fixes @vendor is only required if the id is actually needed. That is, if there is a locally defined AVP whose V flag is set and which does not have a vendor id set by @avp_vendor_id. Also, in the case of an inherited AVP, fix avp_name/2 in a generated dictionary module defaulting vendor id from @vendor in the inheriting dictionary but avp_header/1 defaulting it from the inherited dictionary. In both cases the vendor id now defaults from @vendor in the inherited dictionary. Note that @avp_vendor_id from the inherited dictionary is ignored: any changes from @vendor have to be explicit in the inheriting dictionary. A better alternative to @avp_vendor_id is to simply inherit from dictionaries setting the appropriate @vendor but this was previously somewhat broken so @avp_vendor_id was needed to set the id of an AVP whose definition was copied from another source into a dictionary that only inherited from the common dictionary (which doesn't set V on any AVPs). --- lib/diameter/src/compiler/diameter_codegen.erl | 92 ++++++++++++++++---------- 1 file changed, 58 insertions(+), 34 deletions(-) (limited to 'lib/diameter/src/compiler') diff --git a/lib/diameter/src/compiler/diameter_codegen.erl b/lib/diameter/src/compiler/diameter_codegen.erl index a38cc42df2..b3af326049 100644 --- a/lib/diameter/src/compiler/diameter_codegen.erl +++ b/lib/diameter/src/compiler/diameter_codegen.erl @@ -88,9 +88,6 @@ file(F, Outdir, Mode) -> %% =========================================================================== %% =========================================================================== -choose(true, X, _) -> X; -choose(false, _, X) -> X. - get_value(Key, Plist) -> proplists:get_value(Key, Plist, []). @@ -261,18 +258,26 @@ c_id(error) -> %%% ------------------------------------------------------------------------ f_vendor_id(Spec) -> - {Id, _} = orddict:fetch(vendor, Spec), {?function, vendor_id, 0, - [{?clause, [], [], [?INTEGER(Id)]}]}. + [{?clause, [], [], [vendor_id(orddict:find(vendor, Spec))]}]}. + +vendor_id({ok, {Id, _}}) -> + ?INTEGER(Id); +vendor_id(error) -> + ?APPLY(erlang, error, [?TERM(undefined)]). %%% ------------------------------------------------------------------------ %%% # vendor_name/0 %%% ------------------------------------------------------------------------ f_vendor_name(Spec) -> - {_, Name} = orddict:fetch(vendor, Spec), {?function, vendor_name, 0, - [{?clause, [], [], [?Atom(Name)]}]}. + [{?clause, [], [], [vendor_name(orddict:find(vendor, Spec))]}]}. + +vendor_name({ok, {_, Name}}) -> + ?Atom(Name); +vendor_name(error) -> + ?APPLY(erlang, error, [?TERM(undefined)]). %%% ------------------------------------------------------------------------ %%% # msg_name/1 @@ -364,32 +369,39 @@ f_avp_name(Spec) -> %% allocated by IANA (see Section 11.1). avp_name(Spec) -> - Avps = get_value(avp_types, Spec) - ++ lists:flatmap(fun avps/1, get_value(import_avps, Spec)), - {Vid, _} = orddict:fetch(vendor, Spec), + Avps = get_value(avp_types, Spec), + Imported = get_value(import_avps, Spec), + Vid = orddict:find(vendor, Spec), + Vs = lists:flatmap(fun({V,Ns}) -> [{N,V} || N <- Ns] end, get_value(avp_vendor_id, Spec)), - lists:map(fun(T) -> c_avp_name(T, Vid, Vs) end, Avps) + lists:map(fun(T) -> c_avp_name(T, Vs, Vid) end, Avps) + ++ lists:flatmap(fun(T) -> c_imported_avp_name(T, Vs) end, Imported) ++ [{?clause, [?VAR('_'), ?VAR('_')], [], [?ATOM('AVP')]}]. -c_avp_name({Name, Code, Type, Flags}, Vid, Vs) -> - c_avp_name({?A(Name), ?A(Type)}, - Code, - lists:member('V', Flags), - Vid, - proplists:get_value(Name, Vs)). - -c_avp_name(T, Code, false, _, undefined = U) -> - {?clause, [?INTEGER(Code), ?ATOM(U)], +c_avp_name({Name, Code, Type, Flags}, Vs, Vid) -> + c_avp_name_(?TERM({?A(Name), ?A(Type)}), + ?INTEGER(Code), + vid(Name, Flags, Vs, Vid)). + +%% Note that an imported AVP's vendor id is determined by +%% avp_vendor_id in the inheriting module and vendor in the inherited +%% module. In particular, avp_vendor_id in the inherited module is +%% ignored so can't just call Mod:avp_header/1 to retrieve the vendor +%% id. +c_imported_avp_name({Mod, Avps}, Vs) -> + lists:map(fun(A) -> c_avp_name(A, Vs, {module, Mod}) end, Avps). + +c_avp_name_(T, Code, undefined = U) -> + {?clause, [Code, ?ATOM(U)], [], - [?TERM(T)]}; + [T]}; -c_avp_name(T, Code, true, Vid, V) - when is_integer(Vid) -> - {?clause, [?INTEGER(Code), ?INTEGER(choose(V == undefined, Vid, V))], +c_avp_name_(T, Code, Vid) -> + {?clause, [Code, ?INTEGER(Vid)], [], - [?TERM(T)]}. + [T]}. %%% ------------------------------------------------------------------------ %%% # avp_arity/2 @@ -597,22 +609,26 @@ f_avp_header(Spec) -> avp_header(Spec) -> Native = get_value(avp_types, Spec), Imported = get_value(import_avps, Spec), - {Vid, _} = orddict:fetch(vendor, Spec), + Vid = orddict:find(vendor, Spec), + Vs = lists:flatmap(fun({V,Ns}) -> [{N,V} || N <- Ns] end, get_value(avp_vendor_id, Spec)), - lists:flatmap(fun(A) -> c_avp_header({Vid, Vs}, A) end, + lists:flatmap(fun(A) -> c_avp_header(A, Vs, Vid) end, Native ++ Imported). -c_avp_header({Vid, Vs}, {Name, Code, _Type, Flags}) -> +c_avp_header({Name, Code, _Type, Flags}, Vs, Vid) -> [{?clause, [?Atom(Name)], [], [?TERM({Code, encode_avp_flags(Flags), vid(Name, Flags, Vs, Vid)})]}]; -c_avp_header({_, Vs}, {Mod, Avps}) -> - lists:map(fun(A) -> c_avp_header(Vs, Mod, A) end, Avps). +c_avp_header({Mod, Avps}, Vs, _Vid) -> + lists:map(fun(A) -> c_imported_avp_header(A, Mod, Vs) end, Avps). -c_avp_header(Vs, Mod, {Name, _Code, _Type, Flags}) -> +%% Note that avp_vendor_id in the inherited dictionary is ignored. The +%% value must be changed in the inheriting dictionary. This is +%% consistent with the semantics of avp_name/2. +c_imported_avp_header({Name, _Code, _Type, _Flags}, Mod, Vs) -> Apply = ?APPLY(Mod, avp_header, [?Atom(Name)]), {?clause, [?Atom(Name)], [], @@ -620,7 +636,6 @@ c_avp_header(Vs, Mod, {Name, _Code, _Type, Flags}) -> undefined -> Apply; Vid -> - true = lists:member('V', Flags), %% sanity check ?CALL(setelement, [?INTEGER(3), Apply, ?INTEGER(Vid)]) end]}. @@ -632,10 +647,19 @@ eaf($M, F) -> 2#01000000 bor F; eaf($P, F) -> 2#00100000 bor F. vid(Name, Flags, Vs, Vid) -> - v(lists:member('V', Flags), Name, Vs, Vid). + v(lists:member($V, Flags), Name, Vs, Vid). + +v(true = T, Name, Vs, {module, Mod}) -> + v(T, Name, Vs, {ok, {Mod:vendor_id(), Mod:vendor_name()}}); v(true, Name, Vs, Vid) -> - proplists:get_value(Name, Vs, Vid); + case proplists:get_value(Name, Vs) of + undefined -> + {ok, {Id, _}} = Vid, + Id; + Id -> + Id + end; v(false, _, _, _) -> undefined. -- cgit v1.2.3