From 8a3f090684f0835958e60e6a5e3e81b4d8ff541a Mon Sep 17 00:00:00 2001 From: Vincent de Phily Date: Fri, 9 Jul 2010 17:36:36 +0200 Subject: [PATCH 1/6] erlang: Fix some existing specs and add a few other. dialyzer still complains about dict() and ?assert(false), but I don't think they're real issues. --- erlang/msgpack.erl | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/erlang/msgpack.erl b/erlang/msgpack.erl index ff3eac74..fb9a3e12 100644 --- a/erlang/msgpack.erl +++ b/erlang/msgpack.erl @@ -29,7 +29,7 @@ % erl> c(msgpack). % erl> S = . % erl> {S, <<>>} = msgpack:unpack( msgpack:pack(S) ). --type reason() :: enomem | badarg | no_code_matches | undefined. +-type reason() :: badarg | short. -type msgpack_term() :: [msgpack_term()] | {[{msgpack_term(),msgpack_term()}]} | integer() | float() | binary(). @@ -73,7 +73,7 @@ unpack_all(Data)-> [Term|unpack_all(Binary)] end. --spec pack_map(M::[{msgpack_term(),msgpack_term()}])-> binary() | {error, badarg}. +-spec pack_map(M::[{msgpack_term(),msgpack_term()}]) -> binary() | no_return(). pack_map(M)-> case length(M) of Len when Len < 16 -> @@ -109,8 +109,9 @@ pack_({Map}) when is_list(Map) -> pack_(Map) when is_tuple(Map), element(1,Map)=:=dict -> pack_map(dict:to_list(Map)); pack_(_Other) -> - throw({error, undefined}). + throw({error, badarg}). +-spec pack_uint_(non_neg_integer()) -> binary(). % positive fixnum pack_uint_(N) when N < 128 -> << 2#0:1, N:7 >>; @@ -127,6 +128,7 @@ pack_uint_(N) when N < 16#FFFFFFFF-> pack_uint_(N) -> << 16#CF:8, N:64/big-unsigned-integer-unit:1 >>. +-spec pack_int_(integer()) -> binary(). % negative fixnum pack_int_(N) when N >= -32-> << 2#111:3, N:5 >>; @@ -143,6 +145,7 @@ pack_int_(N) when N > -16#FFFFFFFF -> pack_int_(N) -> << 16#D3:8, N:64/big-signed-integer-unit:1 >>. +-spec pack_double(float()) -> binary(). % float : erlang's float is always IEEE 754 64bit format. % pack_float(F) when is_float(F)-> % << 16#CA:8, F:32/big-float-unit:1 >>. @@ -151,6 +154,7 @@ pack_int_(N) -> pack_double(F) -> << 16#CB:8, F:64/big-float-unit:1 >>. +-spec pack_raw(binary()) -> binary(). % raw bytes pack_raw(Bin) -> case byte_size(Bin) of @@ -162,6 +166,7 @@ pack_raw(Bin) -> << 16#DB:8, Len:32/big-unsigned-integer-unit:1, Bin/binary >> end. +-spec pack_array([msgpack_term()]) -> binary() | no_return(). % list pack_array(L) -> case length(L) of @@ -178,6 +183,7 @@ pack_array_([Head|Tail], Acc) -> pack_array_(Tail, <>). % Users SHOULD NOT send too long list: this uses lists:reverse/1 +-spec unpack_array_(binary(), non_neg_integer(), [msgpack_term()]) -> {[msgpack_term()], binary()} | no_return(). unpack_array_(Bin, 0, Acc) -> {lists:reverse(Acc), Bin}; unpack_array_(Bin, Len, Acc) -> {Term, Rest} = unpack_(Bin), @@ -188,8 +194,8 @@ pack_map_([{Key,Value}|Tail], Acc) -> pack_map_(Tail, << Acc/binary, (pack_(Key))/binary, (pack_(Value))/binary>>). % Users SHOULD NOT send too long list: this uses lists:reverse/1 --spec unpack_map_(binary(), non_neg_integer(), [{msgpack_term(), msgpack_term()}])-> - {[{msgpack_term(), msgpack_term()}], binary()} | no_return(). +-spec unpack_map_(binary(), non_neg_integer(), [{msgpack_term(), msgpack_term()}]) -> + {{[{msgpack_term(), msgpack_term()}]}, binary()} | no_return(). unpack_map_(Bin, 0, Acc) -> {{lists:reverse(Acc)}, Bin}; unpack_map_(Bin, Len, Acc) -> {Key, Rest} = unpack_(Bin), @@ -197,7 +203,7 @@ unpack_map_(Bin, Len, Acc) -> unpack_map_(Rest2, Len-1, [{Key,Value}|Acc]). % unpack them all --spec unpack_(Bin::binary()) -> {msgpack_term(), binary()} | {error, reason()} | no_return(). +-spec unpack_(Bin::binary()) -> {msgpack_term(), binary()} | no_return(). unpack_(Bin) -> case Bin of % ATOMS From 21992f1b9e43f3e0d2c3da8f1f8d94d9ee70b6fb Mon Sep 17 00:00:00 2001 From: Vincent de Phily Date: Fri, 9 Jul 2010 18:53:24 +0200 Subject: [PATCH 2/6] erlang: fix receiving from port_command in unit tests Ports can send data bit by bit; make sure we read all the port has to offer in one go. This should fix the "broken pipe" error we sometime got during testing. We did not previously check the return of compare_all/2, which is why the bug was not noticed. Incidentally, this change fixes dialyzer warnings too. --- erlang/msgpack.erl | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/erlang/msgpack.erl b/erlang/msgpack.erl index fb9a3e12..58ad4143 100644 --- a/erlang/msgpack.erl +++ b/erlang/msgpack.erl @@ -261,6 +261,15 @@ compare_all([LH|LTL], [RH|RTL]) -> ?assertEqual(LH, RH), compare_all(LTL, RTL). +port_receive(Port) -> + port_receive(Port, <<>>). +port_receive(Port, Acc) -> + receive + {Port, {data, Data}} -> port_receive(Port, <>); + {Port, eof} -> Acc + after 1000 -> Acc + end. + test_([]) -> 0; test_([Term|Rest])-> Pack = msgpack:pack(Term), @@ -286,13 +295,12 @@ basic_test()-> Passed = length(Tests). port_test()-> - Port = open_port({spawn, "ruby ../test/crosslang.rb"}, [binary]), Tests = test_data(), - {[Tests],<<>>} = msgpack:unpack(msgpack:pack([Tests])), - true = port_command(Port, msgpack:pack(Tests) ), - receive - {Port, {data, Data}}-> {Tests, <<>>}=msgpack:unpack(Data) - after 1024-> ?assert(false) end, + ?assertEqual({[Tests],<<>>}, msgpack:unpack(msgpack:pack([Tests]))), + + Port = open_port({spawn, "ruby ../test/crosslang.rb"}, [binary, eof]), + true = port_command(Port, msgpack:pack(Tests)), + ?assertEqual({Tests, <<>>}, msgpack:unpack(port_receive(Port))), port_close(Port). test_p(Len,Term,OrigBin,Len) -> @@ -319,7 +327,7 @@ map_test()-> ok. unknown_test()-> - Port = open_port({spawn, "ruby testcase_generator.rb"}, [binary]), + Port = open_port({spawn, "ruby testcase_generator.rb"}, [binary, eof]), Tests = [0, 1, 2, 123, 512, 1230, 678908, -1, -23, -512, -1230, -567898, <<"hogehoge">>, <<"243546rf7g68h798j">>, @@ -331,9 +339,7 @@ unknown_test()-> -234, -50000, 42 ], - receive - {Port, {data, Data}}-> compare_all(Tests, msgpack:unpack_all(Data)) - after 1024-> ?assert(false) end, + ?assertEqual(ok, compare_all(Tests, msgpack:unpack_all(port_receive(Port)))), port_close(Port). other_test()-> From 2c29377abf1997d3a55178ef0562aa38d255f0fe Mon Sep 17 00:00:00 2001 From: Vincent de Phily Date: Fri, 9 Jul 2010 20:30:17 +0200 Subject: [PATCH 3/6] erlang: s/short/incomplete/ and s/badarg/{badarg,Term}/ Nicer error returns. --- erlang/msgpack.erl | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/erlang/msgpack.erl b/erlang/msgpack.erl index 58ad4143..102efed4 100644 --- a/erlang/msgpack.erl +++ b/erlang/msgpack.erl @@ -29,7 +29,7 @@ % erl> c(msgpack). % erl> S = . % erl> {S, <<>>} = msgpack:unpack( msgpack:pack(S) ). --type reason() :: badarg | short. +-type reason() :: {badarg,term()} | incomplete. -type msgpack_term() :: [msgpack_term()] | {[{msgpack_term(),msgpack_term()}]} | integer() | float() | binary(). @@ -43,7 +43,6 @@ pack(Term)-> error:Error when is_tuple(Error), element(1, Error) =:= error -> Error; throw:Exception -> - erlang:display(Exception), {error, Exception} end. @@ -52,9 +51,7 @@ pack(Term)-> % and feed more Bin into this function. % TODO: error case for imcomplete format when short for any type formats. -spec unpack( Bin::binary() )-> {msgpack_term(), binary()} | {error, reason()}. -unpack(Bin) when not is_binary(Bin) -> - {error, badarg}; -unpack(Bin) -> +unpack(Bin) when is_binary(Bin) -> try unpack_(Bin) catch @@ -62,7 +59,9 @@ unpack(Bin) -> Error; throw:Exception -> {error, Exception} - end. + end; +unpack(Other) -> + {error, {badarg, Other}}. -spec unpack_all( binary() ) -> [msgpack_term()]. unpack_all(Data)-> @@ -108,8 +107,8 @@ pack_({Map}) when is_list(Map) -> pack_map(Map); pack_(Map) when is_tuple(Map), element(1,Map)=:=dict -> pack_map(dict:to_list(Map)); -pack_(_Other) -> - throw({error, badarg}). +pack_(Other) -> + throw({error, {badarg, Other}}). -spec pack_uint_(non_neg_integer()) -> binary(). % positive fixnum @@ -241,13 +240,13 @@ unpack_(Bin) -> <<2#1000:4, L:4, Rest/binary>> -> unpack_map_(Rest, L, []); % map % Invalid data - <> when F==16#C1; + <> when F==16#C1; F==16#C4; F==16#C5; F==16#C6; F==16#C7; F==16#C8; F==16#C9; F==16#D4; F==16#D5; F==16#D6; F==16#D7; F==16#D8; F==16#D9 -> - throw(badarg); + throw({badarg, <>}); % Incomplete data (we've covered every complete/invalid case; anything left is incomplete) _ -> - throw(short) + throw(incomplete) end. % ===== test codes ===== % @@ -307,7 +306,7 @@ test_p(Len,Term,OrigBin,Len) -> {Term, <<>>}=msgpack:unpack(OrigBin); test_p(I,_,OrigBin,Len) when I < Len-> <> = OrigBin, - ?assertEqual({error,short}, msgpack:unpack(Bin)). + ?assertEqual({error,incomplete}, msgpack:unpack(Bin)). partial_test()-> % error handling test. Term = lists:seq(0, 45), @@ -343,7 +342,7 @@ unknown_test()-> port_close(Port). other_test()-> - ?assertEqual({error,short},msgpack:unpack(<<>>)). + ?assertEqual({error,incomplete},msgpack:unpack(<<>>)). benchmark_test()-> Data=[test_data() || _ <- lists:seq(0, 10000)], From 02c882bda39f65bdbe7be1b149b74f0f9a8283c6 Mon Sep 17 00:00:00 2001 From: Vincent de Phily Date: Fri, 9 Jul 2010 20:34:38 +0200 Subject: [PATCH 4/6] erlang: Make pack_map/1 api private --- erlang/msgpack.erl | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/erlang/msgpack.erl b/erlang/msgpack.erl index 102efed4..0e7fb5d4 100644 --- a/erlang/msgpack.erl +++ b/erlang/msgpack.erl @@ -23,7 +23,6 @@ %% APIs are almost compatible with C API (http://msgpack.sourceforge.jp/c:doc) %% except buffering functions (both copying and zero-copying). -export([pack/1, unpack/1, unpack_all/1]). --export([pack_map/1]). % compile: % erl> c(msgpack). @@ -72,16 +71,6 @@ unpack_all(Data)-> [Term|unpack_all(Binary)] end. --spec pack_map(M::[{msgpack_term(),msgpack_term()}]) -> binary() | no_return(). -pack_map(M)-> - case length(M) of - Len when Len < 16 -> - << 2#1000:4, Len:4/integer-unit:1, (pack_map_(M, <<>>))/binary >>; - Len when Len < 16#10000 -> % 65536 - << 16#DE:8, Len:16/big-unsigned-integer-unit:1, (pack_map_(M, <<>>))/binary >>; - Len -> - << 16#DF:8, Len:32/big-unsigned-integer-unit:1, (pack_map_(M, <<>>))/binary >> - end. % ===== internal APIs ===== % @@ -188,6 +177,17 @@ unpack_array_(Bin, Len, Acc) -> {Term, Rest} = unpack_(Bin), unpack_array_(Rest, Len-1, [Term|Acc]). +-spec pack_map(M::[{msgpack_term(),msgpack_term()}]) -> binary() | no_return(). +pack_map(M)-> + case length(M) of + Len when Len < 16 -> + << 2#1000:4, Len:4/integer-unit:1, (pack_map_(M, <<>>))/binary >>; + Len when Len < 16#10000 -> % 65536 + << 16#DE:8, Len:16/big-unsigned-integer-unit:1, (pack_map_(M, <<>>))/binary >>; + Len -> + << 16#DF:8, Len:32/big-unsigned-integer-unit:1, (pack_map_(M, <<>>))/binary >> + end. + pack_map_([], Acc) -> Acc; pack_map_([{Key,Value}|Tail], Acc) -> pack_map_(Tail, << Acc/binary, (pack_(Key))/binary, (pack_(Value))/binary>>). From e944c1ee93083a054f318e42a16eff699d9f066d Mon Sep 17 00:00:00 2001 From: Vincent de Phily Date: Fri, 9 Jul 2010 20:37:06 +0200 Subject: [PATCH 5/6] erlang: Only handle throw() in pack/1 and unpack/1 Rationale: We only use throw/1 for error handling, never erlang:error/1. Caller bugs will get a nice {error,...} return while library bugs will bubble up in all their uglyness; that's the proper way to do things in erlang. --- erlang/msgpack.erl | 4 ---- 1 file changed, 4 deletions(-) diff --git a/erlang/msgpack.erl b/erlang/msgpack.erl index 0e7fb5d4..f23ac878 100644 --- a/erlang/msgpack.erl +++ b/erlang/msgpack.erl @@ -39,8 +39,6 @@ pack(Term)-> try pack_(Term) catch - error:Error when is_tuple(Error), element(1, Error) =:= error -> - Error; throw:Exception -> {error, Exception} end. @@ -54,8 +52,6 @@ unpack(Bin) when is_binary(Bin) -> try unpack_(Bin) catch - error:Error when is_tuple(Error), element(1, Error) =:= error -> - Error; throw:Exception -> {error, Exception} end; From e629e8784ff242f7c5e62066b0670f5d82afe4e2 Mon Sep 17 00:00:00 2001 From: Vincent de Phily Date: Mon, 12 Jul 2010 14:08:22 +0200 Subject: [PATCH 6/6] erlang: Improve documentation The doc is in edoc format, generated from the source as an html file. The makefile's default action now also generates the documentation. I ignored unpack_all/1 and pack(dict()) for now because their future is still uncertain. --- erlang/OMakefile | 7 +++-- erlang/msgpack.erl | 70 +++++++++++++++++++++++++++++++++------------- 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/erlang/OMakefile b/erlang/OMakefile index 34c590f0..89b1c63f 100644 --- a/erlang/OMakefile +++ b/erlang/OMakefile @@ -30,13 +30,16 @@ # If so, define the subdirectory targets and uncomment this section. # -.DEFAULT: msgpack.beam +.DEFAULT: msgpack.beam msgpack.html msgpack.beam: msgpack.erl erlc $< +msgpack.html: msgpack.erl + erl -noshell -run edoc_run file $< + test: msgpack.beam erl -noshell -s msgpack test -s init stop clean: - -rm *.beam + -rm -f *.beam *.html diff --git a/erlang/msgpack.erl b/erlang/msgpack.erl index f23ac878..aa9851da 100644 --- a/erlang/msgpack.erl +++ b/erlang/msgpack.erl @@ -15,26 +15,47 @@ %% See the License for the specific language governing permissions and %% limitations under the License. + +%% @doc MessagePack codec for Erlang. +%% +%% APIs are almost compatible with C API +%% except for buffering functions (both copying and zero-copying), which are unavailable. +%% +%% +%% +%% +%% +%% +%% +%% +%% +%% +%% +%%
Equivalence between Erlang and Msgpack type :
erlang msgpack
integer() pos_fixnum/neg_fixnum/uint8/uint16/uint32/uint64/int8/int16/int32/int64
float() float/double
nil nil
boolean() boolean
binary() fix_raw/raw16/raw32
list() fix_array/array16/array32
{proplist()} fix_map/map16/map32
+%% @end + -module(msgpack). -author('kuenishi+msgpack@gmail.com'). -%% tuples, atoms are not supported. lists, integers, double, and so on. -%% see http://msgpack.sourceforge.jp/spec for supported formats. -%% APIs are almost compatible with C API (http://msgpack.sourceforge.jp/c:doc) -%% except buffering functions (both copying and zero-copying). -export([pack/1, unpack/1, unpack_all/1]). -% compile: -% erl> c(msgpack). -% erl> S = . -% erl> {S, <<>>} = msgpack:unpack( msgpack:pack(S) ). --type reason() :: {badarg,term()} | incomplete. +% @type msgpack_term() = [msgpack_term()] +% | {[{msgpack_term(),msgpack_term()}]} +% | integer() | float() | binary(). +% Erlang representation of msgpack data. -type msgpack_term() :: [msgpack_term()] | {[{msgpack_term(),msgpack_term()}]} | integer() | float() | binary(). -% ===== external APIs ===== % --spec pack(Term::msgpack_term()) -> binary() | {error, reason()}. + +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +% external APIs +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% + +% @doc Encode an erlang term into an msgpack binary. +% Returns {error, {badarg, term()}} if the input is illegal. +% @spec pack(Term::msgpack_term()) -> binary() | {error, {badarg, term()}} +-spec pack(Term::msgpack_term()) -> binary() | {error, {badarg, term()}}. pack(Term)-> try pack_(Term) @@ -43,11 +64,12 @@ pack(Term)-> {error, Exception} end. -% unpacking. -% if failed in decoding and not end, get more data -% and feed more Bin into this function. -% TODO: error case for imcomplete format when short for any type formats. --spec unpack( Bin::binary() )-> {msgpack_term(), binary()} | {error, reason()}. +% @doc Decode an msgpack binary into an erlang term. +% It only decodes the first msgpack packet contained in the binary; the rest is returned as is. +% Returns {error, {badarg, term()}} if the input is corrupted. +% Returns {error, incomplete} if the input is not a full msgpack packet (caller should gather more data and try again). +% @spec unpack(Bin::binary()) -> {msgpack_term(), binary()} | {error, incomplete} | {error, {badarg, term()}} +-spec unpack(Bin::binary()) -> {msgpack_term(), binary()} | {error, incomplete} | {error, {badarg, term()}}. unpack(Bin) when is_binary(Bin) -> try unpack_(Bin) @@ -58,7 +80,7 @@ unpack(Bin) when is_binary(Bin) -> unpack(Other) -> {error, {badarg, Other}}. --spec unpack_all( binary() ) -> [msgpack_term()]. +-spec unpack_all(binary()) -> [msgpack_term()]. unpack_all(Data)-> case unpack(Data) of { Term, Binary } when bit_size(Binary) =:= 0 -> @@ -68,7 +90,9 @@ unpack_all(Data)-> end. -% ===== internal APIs ===== % +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +% internal APIs +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% % pack them all -spec pack_(msgpack_term()) -> binary() | no_return(). @@ -95,6 +119,7 @@ pack_(Map) when is_tuple(Map), element(1,Map)=:=dict -> pack_(Other) -> throw({error, {badarg, Other}}). + -spec pack_uint_(non_neg_integer()) -> binary(). % positive fixnum pack_uint_(N) when N < 128 -> @@ -129,6 +154,7 @@ pack_int_(N) when N > -16#FFFFFFFF -> pack_int_(N) -> << 16#D3:8, N:64/big-signed-integer-unit:1 >>. + -spec pack_double(float()) -> binary(). % float : erlang's float is always IEEE 754 64bit format. % pack_float(F) when is_float(F)-> @@ -138,6 +164,7 @@ pack_int_(N) -> pack_double(F) -> << 16#CB:8, F:64/big-float-unit:1 >>. + -spec pack_raw(binary()) -> binary(). % raw bytes pack_raw(Bin) -> @@ -150,6 +177,7 @@ pack_raw(Bin) -> << 16#DB:8, Len:32/big-unsigned-integer-unit:1, Bin/binary >> end. + -spec pack_array([msgpack_term()]) -> binary() | no_return(). % list pack_array(L) -> @@ -173,6 +201,7 @@ unpack_array_(Bin, Len, Acc) -> {Term, Rest} = unpack_(Bin), unpack_array_(Rest, Len-1, [Term|Acc]). + -spec pack_map(M::[{msgpack_term(),msgpack_term()}]) -> binary() | no_return(). pack_map(M)-> case length(M) of @@ -245,7 +274,10 @@ unpack_(Bin) -> throw(incomplete) end. -% ===== test codes ===== % + +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +% unit tests +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% -include_lib("eunit/include/eunit.hrl"). -ifdef(EUNIT).