Merge bug17162.

3 files changed, 109 insertions, 121 deletions

diff --git a/codegen.py b/codegen.py
index 7636c196..494be73d 100644
--- a/codegen.py
+++ b/codegen.py
@@ -42,7 +42,8 @@ erlangTypeMap = {
 def convertTable(d):
     if len(d) == 0:
         return "[]"
-    else: raise 'Non-empty table defaults not supported', d
+    else:
+        raise Exception('Non-empty table defaults not supported ' + d)
 
 erlangDefaultValueTypeConvMap = {
     bool : lambda x: str(x).lower(),
@@ -229,9 +230,32 @@ def genErl(spec):
         print "  %s;" % (recordConstructorExpr,)
 
     def genDecodeProperties(c):
-        print "decode_properties(%d, PropBin) ->" % (c.index)
-        print "  %s = rabbit_binary_parser:parse_properties(%s, PropBin)," % \
-              (fieldTempList(c.fields), fieldTypeList(c.fields))
+        def presentBin(fields):
+            ps = ', '.join(['P' + str(f.index) + ':1' for f in fields])
+            return '<<' + ps + ', _:%d, R0/binary>>' % (16 - len(fields),)
+        def mkMacroName(field):
+            return '?' + field.domain.upper() + '_PROP'
+        def writePropFieldLine(field, bin_next = None):
+            i = str(field.index)
+            if not bin_next:
+                bin_next = 'R' + str(field.index + 1)
+            if field.domain in ['octet', 'timestamp']:
+                print ("  {%s, %s} = %s(%s, %s, %s, %s)," %
+                       ('F' + i, bin_next, mkMacroName(field), 'P' + i,
+                        'R' + i, 'I' + i, 'X' + i))
+            else:
+                print ("  {%s, %s} = %s(%s, %s, %s, %s, %s)," %
+                       ('F' + i, bin_next, mkMacroName(field), 'P' + i,
+                        'R' + i, 'L' + i, 'S' + i, 'X' + i))
+
+        if len(c.fields) == 0:
+            print "decode_properties(%d, _) ->" % (c.index,)
+        else:
+            print ("decode_properties(%d, %s) ->" %
+                   (c.index, presentBin(c.fields)))
+            for field in c.fields[:-1]:
+                writePropFieldLine(field)
+            writePropFieldLine(c.fields[-1], "<<>>")
         print "  #'P_%s'{%s};" % (erlangize(c.name), fieldMapList(c.fields))
 
     def genFieldPreprocessing(packed):
@@ -272,7 +296,7 @@ def genErl(spec):
         if mCls == 'SOFT_ERROR': genLookupException1(c,'false')
         elif mCls == 'HARD_ERROR': genLookupException1(c, 'true')
         elif mCls == '': pass
-        else: raise 'Unknown constant class', cls
+        else: raise Exception('Unknown constant class' + cls)
 
     def genLookupException1(c,hardErrorBoolStr):
         n = erlangConstantName(c)
@@ -404,6 +428,27 @@ shortstr_size(S) ->
         Len when Len =< 255 -> Len;
         _                   -> exit(method_field_shortstr_overflow)
     end.
+
+-define(SHORTSTR_PROP(P, R, L, S, X),
+        if P =:= 0 -> {undefined, R};
+           true    -> <<L:8/unsigned, S:L/binary, X/binary>> = R,
+                      {S, X}
+        end).
+-define(TABLE_PROP(P, R, L, T, X),
+        if P =:= 0 -> {undefined, R};
+           true    -> <<L:32/unsigned, T:L/binary, X/binary>> = R,
+                      {rabbit_binary_parser:parse_table(T), X}
+        end).
+-define(OCTET_PROP(P, R, I, X),
+        if P =:= 0 -> {undefined, R};
+           true    -> <<I:8/unsigned, X/binary>> = R,
+                      {I, X}
+        end).
+-define(TIMESTAMP_PROP(P, R, I, X),
+        if P =:= 0 -> {undefined, R};
+           true    -> <<I:64/unsigned, X/binary>> = R,
+                      {I, X}
+        end).
 """
     version = "{%d, %d, %d}" % (spec.major, spec.minor, spec.revision)
     if version == '{8, 0, 0}': version = '{0, 8, 0}'
diff --git a/src/rabbit_binary_parser.erl b/src/rabbit_binary_parser.erl
index 88026bab..f3ca4e98 100644
--- a/src/rabbit_binary_parser.erl
+++ b/src/rabbit_binary_parser.erl
@@ -18,7 +18,7 @@
 
 -include("rabbit.hrl").
 
--export([parse_table/1, parse_properties/2]).
+-export([parse_table/1]).
 -export([ensure_content_decoded/1, clear_decoded_content/1]).
 
 %%----------------------------------------------------------------------------
@@ -26,8 +26,6 @@
 -ifdef(use_specs).
 
 -spec(parse_table/1 :: (binary()) -> rabbit_framing:amqp_table()).
--spec(parse_properties/2 ::
-        ([rabbit_framing:amqp_property_type()], binary()) -> [any()]).
 -spec(ensure_content_decoded/1 ::
         (rabbit_types:content()) -> rabbit_types:decoded_content()).
 -spec(clear_decoded_content/1 ::
@@ -94,57 +92,6 @@ parse_field_value(<<"x", VLen:32/unsigned, ValueString:VLen/binary, Rest/binary>
 parse_field_value(<<"V", Rest/binary>>) ->
     {void, undefined, Rest}.
 
-
-parse_properties([], _PropBin) ->
-    [];
-parse_properties(TypeList, PropBin) ->
-    FlagCount = length(TypeList),
-    %% round up to the nearest multiple of 15 bits, since the 16th bit
-    %% in each short is a "continuation" bit.
-    FlagsLengthBytes = trunc((FlagCount + 14) / 15) * 2,
-    <<Flags:FlagsLengthBytes/binary, Properties/binary>> = PropBin,
-    <<FirstShort:16, Remainder/binary>> = Flags,
-    parse_properties(0, TypeList, [], FirstShort, Remainder, Properties).
-
-parse_properties(_Bit, [], Acc, _FirstShort,
-                 _Remainder, <<>>) ->
-    lists:reverse(Acc);
-parse_properties(_Bit, [], _Acc, _FirstShort,
-                 _Remainder, _LeftoverBin) ->
-    exit(content_properties_binary_overflow);
-parse_properties(15, TypeList, Acc, _OldFirstShort,
-                 <<NewFirstShort:16,Remainder/binary>>, Properties) ->
-    parse_properties(0, TypeList, Acc, NewFirstShort, Remainder, Properties);
-parse_properties(Bit, [Type | TypeListRest], Acc, FirstShort,
-                 Remainder, Properties) ->
-    {Value, Rest} =
-        if (FirstShort band (1 bsl (15 - Bit))) /= 0 ->
-                parse_property(Type, Properties);
-           Type == bit -> {false    , Properties};
-           true        -> {undefined, Properties}
-        end,
-    parse_properties(Bit + 1, TypeListRest, [Value | Acc], FirstShort,
-                     Remainder, Rest).
-
-parse_property(shortstr, <<Len:8/unsigned, String:Len/binary, Rest/binary>>) ->
-    {String, Rest};
-parse_property(longstr, <<Len:32/unsigned, String:Len/binary, Rest/binary>>) ->
-    {String, Rest};
-parse_property(octet, <<Int:8/unsigned, Rest/binary>>) ->
-    {Int, Rest};
-parse_property(shortint, <<Int:16/unsigned, Rest/binary>>) ->
-    {Int, Rest};
-parse_property(longint, <<Int:32/unsigned, Rest/binary>>) ->
-    {Int, Rest};
-parse_property(longlongint, <<Int:64/unsigned, Rest/binary>>) ->
-    {Int, Rest};
-parse_property(timestamp, <<Int:64/unsigned, Rest/binary>>) ->
-    {Int, Rest};
-parse_property(bit, Rest) ->
-    {true, Rest};
-parse_property(table, <<Len:32/unsigned, Table:Len/binary, Rest/binary>>) ->
-    {parse_table(Table), Rest}.
-
 ensure_content_decoded(Content = #content{properties = Props})
   when Props =/= none ->
     Content;
diff --git a/src/rabbit_tests.erl b/src/rabbit_tests.erl
index bd89d54d..3a4f6f84 100644
--- a/src/rabbit_tests.erl
+++ b/src/rabbit_tests.erl
@@ -30,12 +30,6 @@
 -define(TRANSIENT_MSG_STORE,  msg_store_transient).
 -define(CLEANUP_QUEUE_NAME, <<"cleanup-queue">>).
 
-test_content_prop_roundtrip(Datum, Binary) ->
-    Types =  [element(1, E) || E <- Datum],
-    Values = [element(2, E) || E <- Datum],
-    Values = rabbit_binary_parser:parse_properties(Types, Binary), %% assertion
-    Binary = rabbit_binary_generator:encode_properties(Types, Values). %% assertion
-
 all_tests() ->
     passed = gm_tests:all_tests(),
     passed = mirrored_supervisor_tests:all_tests(),
@@ -306,67 +300,69 @@ test_parsing() ->
     passed = test_field_values(),
     passed.
 
+test_content_prop_encoding(Datum, Binary) ->
+    Types =  [element(1, E) || E <- Datum],
+    Values = [element(2, E) || E <- Datum],
+    Binary = rabbit_binary_generator:encode_properties(Types, Values). %% assertion
+
 test_content_properties() ->
-    test_content_prop_roundtrip([], <<0, 0>>),
-    test_content_prop_roundtrip([{bit, true}, {bit, false}, {bit, true}, {bit, false}],
-                                <<16#A0, 0>>),
-    test_content_prop_roundtrip([{bit, true}, {octet, 123}, {bit, true}, {octet, undefined},
-                                 {bit, true}],
-                                <<16#E8,0,123>>),
-    test_content_prop_roundtrip([{bit, true}, {octet, 123}, {octet, 123}, {bit, true}],
-                                <<16#F0,0,123,123>>),
-    test_content_prop_roundtrip([{bit, true}, {shortstr, <<"hi">>}, {bit, true},
-                                 {shortint, 54321}, {bit, true}],
-                                <<16#F8,0,2,"hi",16#D4,16#31>>),
-    test_content_prop_roundtrip([{bit, true}, {shortstr, undefined}, {bit, true},
-                                 {shortint, 54321}, {bit, true}],
-                                <<16#B8,0,16#D4,16#31>>),
-    test_content_prop_roundtrip([{table, [{<<"a signedint">>, signedint, 12345678},
-                                          {<<"a longstr">>, longstr, <<"yes please">>},
-                                          {<<"a decimal">>, decimal, {123, 12345678}},
-                                          {<<"a timestamp">>, timestamp, 123456789012345},
-                                          {<<"a nested table">>, table,
-                                           [{<<"one">>, signedint, 1},
-                                            {<<"two">>, signedint, 2}]}]}],
-                                <<
-                                  %% property-flags
-                                  16#8000:16,
-
-                                  %% property-list:
-
-                                  %% table
-                                  117:32,                % table length in bytes
-
-                                  11,"a signedint",      % name
-                                  "I",12345678:32,       % type and value
-
-                                  9,"a longstr",
-                                  "S",10:32,"yes please",
-
-                                  9,"a decimal",
-                                  "D",123,12345678:32,
-
-                                  11,"a timestamp",
-                                  "T", 123456789012345:64,
-
-                                  14,"a nested table",
-                                  "F",
-                                  18:32,
-
-                                  3,"one",
-                                  "I",1:32,
-
-                                  3,"two",
-                                  "I",2:32 >>),
-    case catch rabbit_binary_parser:parse_properties([bit, bit, bit, bit], <<16#A0,0,1>>) of
-        {'EXIT', content_properties_binary_overflow} -> passed;
-        V -> exit({got_success_but_expected_failure, V})
-    end.
+    test_content_prop_encoding([], <<0, 0>>),
+    test_content_prop_encoding([{bit, true}, {bit, false}, {bit, true}, {bit, false}],
+                               <<16#A0, 0>>),
+    test_content_prop_encoding([{bit, true}, {octet, 123}, {bit, true}, {octet, undefined},
+                                {bit, true}],
+                               <<16#E8,0,123>>),
+    test_content_prop_encoding([{bit, true}, {octet, 123}, {octet, 123}, {bit, true}],
+                               <<16#F0,0,123,123>>),
+    test_content_prop_encoding([{bit, true}, {shortstr, <<"hi">>}, {bit, true},
+                                {shortint, 54321}, {bit, true}],
+                               <<16#F8,0,2,"hi",16#D4,16#31>>),
+    test_content_prop_encoding([{bit, true}, {shortstr, undefined}, {bit, true},
+                                {shortint, 54321}, {bit, true}],
+                               <<16#B8,0,16#D4,16#31>>),
+    test_content_prop_encoding([{table, [{<<"a signedint">>, signedint, 12345678},
+                                         {<<"a longstr">>, longstr, <<"yes please">>},
+                                         {<<"a decimal">>, decimal, {123, 12345678}},
+                                         {<<"a timestamp">>, timestamp, 123456789012345},
+                                         {<<"a nested table">>, table,
+                                          [{<<"one">>, signedint, 1},
+                                           {<<"two">>, signedint, 2}]}]}],
+                               <<
+                                 %% property-flags
+                                 16#8000:16,
+
+                                 %% property-list:
+
+                                 %% table
+                                 117:32,                % table length in bytes
+
+                                 11,"a signedint",      % name
+                                 "I",12345678:32,       % type and value
+
+                                 9,"a longstr",
+                                 "S",10:32,"yes please",
+
+                                 9,"a decimal",
+                                 "D",123,12345678:32,
+
+                                 11,"a timestamp",
+                                 "T", 123456789012345:64,
+
+                                 14,"a nested table",
+                                 "F",
+                                 18:32,
+
+                                 3,"one",
+                                 "I",1:32,
+
+                                 3,"two",
+                                 "I",2:32 >>),
+    passed.
 
 test_field_values() ->
     %% FIXME this does not test inexact numbers (double and float) yet,
     %% because they won't pass the equality assertions
-    test_content_prop_roundtrip(
+    test_content_prop_encoding(
       [{table, [{<<"longstr">>, longstr, <<"Here is a long string">>},
                 {<<"signedint">>, signedint, 12345},
                 {<<"decimal">>, decimal, {3, 123456}},