[thrift] iolists and binaries in Erlang

Summary: three related changes:
 * for numeric types in tBinaryProtocol call effectful_write with binary(), not [char()]
 * tBinaryProtocol:writeString now takes either [char()], iolist(), or binary() ... now you can use any of the three where a Thrift string is required
 * tBufferedTransport now buffers by building an iolist() rather than Buffer++Data.  zomg.

Reviewed By: eletuchy

Test Plan: everything seems to work as usual, and binary() works as expected

Revert Plan: ok

git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665447 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/lib/erl/src/protocol/tBinaryProtocol.erl b/lib/erl/src/protocol/tBinaryProtocol.erl
index efcfd77..cc71684 100644 (file)
--- a/lib/erl/src/protocol/tBinaryProtocol.erl
+++ b/lib/erl/src/protocol/tBinaryProtocol.erl
@@ -105,36 +105,41 @@ writeSetBegin(This, Etype, Size) ->
 
 %
 
-writeBool(This, Bool) ->
-    case Bool of
-        true  -> ?L1(writeByte, 1);
-        false -> ?L1(writeByte, 0)
-    end.
+writeBool(This, true) ->
+    ?L1(writeByte, 1);
+writeBool(This, false) ->
+    ?L1(writeByte, 0).
+
+writeByte(This, Byte) when is_integer(Byte) ->
+    Trans = oop:get(This, trans),
+    ?R1(Trans, effectful_write, <<Byte:8/big>>).
 
-writeByte(This, Byte) ->
+writeI16(This, I16) when is_integer(I16) ->
     Trans = oop:get(This, trans),
-    ?R1(Trans, effectful_write, binary_to_list(<<Byte:8/big>>)).
+    ?R1(Trans, effectful_write, <<I16:16/big>>).
 
-writeI16(This, I16) ->
+writeI32(This, I32) when is_integer(I32) ->
     Trans = oop:get(This, trans),
-    ?R1(Trans, effectful_write, binary_to_list(<<I16:16/big>>)).
+    ?R1(Trans, effectful_write, <<I32:32/big>>).
 
-writeI32(This, I32) ->
+writeI64(This, I64) when is_integer(I64) ->
     Trans = oop:get(This, trans),
-    ?R1(Trans, effectful_write, binary_to_list(<<I32:32/big>>)).
+    ?R1(Trans, effectful_write, <<I64:64/big>>).
 
-writeI64(This, I64) ->
+writeDouble(This, Double) when is_float(Double) ->
     Trans = oop:get(This, trans),
-    ?R1(Trans, effectful_write, binary_to_list(<<I64:64/big>>)).
+    ?R1(Trans, effectful_write, <<Double:64/big>>).
 
-writeDouble(This, Double) ->
+writeString(This, Str) when is_list(Str) -> % [char()] or iolist()
     Trans = oop:get(This, trans),
-    ?R1(Trans, effectful_write, binary_to_list(<<Double:64/big>>)).
+    Data = list_to_binary(Str),
+    ?L1(writeI32, size(Data)),
+    ?R1(Trans, effectful_write, Data);
 
-writeString(This, Str) ->
+writeString(This, Binary) when is_binary(Binary) ->
     Trans = oop:get(This, trans),
-    ?L1(writeI32, length(Str)),
-    ?R1(Trans, effectful_write, Str).
+    ?L1(writeI32, size(Binary)),
+    ?R1(Trans, effectful_write, Binary).
 
 %%
 

diff --git a/lib/erl/src/transport/tBufferedTransport.erl b/lib/erl/src/transport/tBufferedTransport.erl
index 18d9af9..2b3e0aa 100644 (file)
--- a/lib/erl/src/transport/tBufferedTransport.erl
+++ b/lib/erl/src/transport/tBufferedTransport.erl
@@ -72,9 +72,9 @@ read(This, Sz) ->
     Transport = oop:get(This, transport),
     ?R1(Transport, read, Sz).
 
-effectful_write(This, Buf) -> % be sure to rebind This to the retval
+effectful_write(This, Data) -> % be sure to rebind This to the retval
     Wbuf = oop:get(This, wbuf),
-    This1 = oop:set(This, wbuf, Wbuf++Buf), % TODO: ++ efficiency?
+    This1 = oop:set(This, wbuf, [Wbuf, Data]), % build an iolist()
     {ok, This1}.
 
 effectful_flush(This) ->
@@ -82,5 +82,5 @@ effectful_flush(This) ->
     Transport = oop:get(This, transport),
     ?R1(Transport, effectful_write, Wbuf),
     ?R0(Transport, effectful_flush),
-    This1 = oop:set(This, wbuf, ""),
+    This1 = oop:set(This, wbuf, []),
     {ok, This1}.

diff --git a/lib/erl/src/transport/tSocket.erl b/lib/erl/src/transport/tSocket.erl
index 1792893..f23cdee 100644 (file)
--- a/lib/erl/src/transport/tSocket.erl
+++ b/lib/erl/src/transport/tSocket.erl
@@ -85,16 +85,10 @@ effectful_open(This) ->
 isOpen(This) ->
     oop:get(This, handle) /= nil.
 
-effectful_write(This, Str) ->
+effectful_write(This, Data) ->
     Handle = oop:get(This, handle),
-    Val = gen_tcp:send(Handle, Str),
 
-    %% DEBUG
-    %% error_logger:info_msg("tSocket: wrote ~p~n", [Str]),
-
-    %% error_logger:info_msg("WRITE |~p| (~p)", [Str,Val]),
-
-    case Val of
+    case gen_tcp:send(Handle, Data) of
         {error, _} ->
             tException:throw(tTransportException, [?tTransportException_NOT_OPEN, "in write"]);
         ok ->