Refactor to use only one decompose_from_binary/1 variant

Plus address cosmetic review feedback.

(cherry picked from commit 81e0f64)

Author: michaelklishin

deps/rabbit/src/rabbit_pid_codec.erl

@@ -9,7 +9,6 @@
 
 -export([decompose/1,
          decompose_from_binary/1,
-         decompose_from_binary_safe/1,
          recompose/1,
          recompose_to_binary/1]).
 
@@ -33,36 +32,31 @@
 -spec decompose(pid()) -> decomposed_pid().
 decompose(Pid) ->
     Bin = term_to_binary(Pid, [{minor_version, 2}]),
-    decompose_from_binary(Bin).
+    {ok, #{node := NodeBin} = Parts} = decompose_from_binary(Bin),
+    %% Safe: the input is a real local pid, so its node atom always exists.
+    Parts#{node := binary_to_existing_atom(NodeBin, utf8)}.
 
--spec decompose_from_binary(binary()) -> decomposed_pid().
-decompose_from_binary(Bin) ->
-    <<?TTB_PREFIX, ?NEW_PID_EXT, PidData/binary>> = Bin,
-    {Node, Rest} = case PidData of
-                       <<?ATOM_UTF8_EXT, Len:16/integer, Node0:Len/binary, Rest1/binary>> ->
-                           {Node0, Rest1};
-                       <<?SMALL_ATOM_UTF8_EXT, Len/integer, Node0:Len/binary, Rest1/binary>> ->
-                           {Node0, Rest1}
-                   end,
-    <<ID:32/integer, Serial:32/integer, Creation:32/integer>> = Rest,
-    #{node => binary_to_atom(Node, utf8),
-      id => ID,
-      serial => Serial,
-      creation => Creation}.
-
-%% Returns the node as a binary and returns 'error' on malformed input.
--spec decompose_from_binary_safe(binary()) -> {ok, decomposed_pid_bin()} | error.
-decompose_from_binary_safe(<<?TTB_PREFIX, ?NEW_PID_EXT,
-                             ?ATOM_UTF8_EXT, Len:16/integer, Node:Len/binary,
-                             ID:32/integer, Serial:32/integer,
-                             Creation:32/integer>>) ->
-    {ok, #{node => Node, id => ID, serial => Serial, creation => Creation}};
-decompose_from_binary_safe(<<?TTB_PREFIX, ?NEW_PID_EXT,
-                             ?SMALL_ATOM_UTF8_EXT, Len:8/integer, Node:Len/binary,
-                             ID:32/integer, Serial:32/integer,
-                             Creation:32/integer>>) ->
-    {ok, #{node => Node, id => ID, serial => Serial, creation => Creation}};
-decompose_from_binary_safe(_) ->
+%% Returns the node name as a binary and 'error' on malformed input.
+-spec decompose_from_binary(binary()) -> {ok, decomposed_pid_bin()} | error.
+decompose_from_binary(<<?TTB_PREFIX, ?NEW_PID_EXT,
+                        ?ATOM_UTF8_EXT, Len:16/integer, Node:Len/binary,
+                        ID:32/integer, Serial:32/integer,
+                        Creation:32/integer>>) ->
+    {ok,
+      #{node => Node,
+        id => ID,
+        serial => Serial,
+        creation => Creation}};
+decompose_from_binary(<<?TTB_PREFIX, ?NEW_PID_EXT,
+                        ?SMALL_ATOM_UTF8_EXT, Len:8/integer, Node:Len/binary,
+                        ID:32/integer, Serial:32/integer,
+                        Creation:32/integer>>) ->
+    {ok,
+      #{node => Node,
+        id => ID,
+        serial => Serial,
+        creation => Creation}};
+decompose_from_binary(_) ->
     error.
 
 -spec recompose_to_binary(decomposed_pid()) -> binary().

deps/rabbit/src/rabbit_volatile_queue.erl

@@ -377,7 +377,7 @@ pid_from_name(<<?PREFIX, Bin/binary>>, CandidateNodes) ->
     try
         [PidBase64, _KeyBase64] = binary:split(Bin, Cp),
         PidBin = base64:decode(PidBase64),
-        case rabbit_pid_codec:decompose_from_binary_safe(PidBin) of
+        case rabbit_pid_codec:decompose_from_binary(PidBin) of
             {ok, #{node := ShortenedNodename} = PidParts0} ->
                 NodeHash = node_hash_from_binary(ShortenedNodename),
                 case maps:get(NodeHash, CandidateNodes, undefined) of

deps/rabbit/test/pid_codec_SUITE.erl

@@ -35,13 +35,13 @@ end_per_suite(_Config) -> ok.
 safe_decoder_roundtrip_self(_) ->
     Pid = self(),
     Bin = term_to_binary(Pid, [{minor_version, 2}]),
-    {ok, Parts} = rabbit_pid_codec:decompose_from_binary_safe(Bin),
+    {ok, Parts} = rabbit_pid_codec:decompose_from_binary(Bin),
     Recomposed = rabbit_pid_codec:recompose(Parts#{node := node()}),
     ?assertEqual(Pid, Recomposed).
 
 safe_decoder_returns_binary_node(_) ->
     Bin = term_to_binary(self(), [{minor_version, 2}]),
-    {ok, #{node := NodeBin}} = rabbit_pid_codec:decompose_from_binary_safe(Bin),
+    {ok, #{node := NodeBin}} = rabbit_pid_codec:decompose_from_binary(Bin),
     ?assert(is_binary(NodeBin)),
     ?assertEqual(atom_to_binary(node()), NodeBin).
 
@@ -67,20 +67,20 @@ safe_decoder_rejects_malformed(_) ->
              <<?TTB_PREFIX, ?NEW_PID_EXT,
                ?SMALL_ATOM_UTF8_EXT, 1:8, "x", 0:32, 0:32, 0:24>>
             ],
-    [?assertEqual(error, rabbit_pid_codec:decompose_from_binary_safe(C))
+    [?assertEqual(error, rabbit_pid_codec:decompose_from_binary(C))
      || C <- Cases].
 
 safe_decoder_no_atoms_for_unknown_node_names(_) ->
     Inputs = [{fresh_node_bytes(I), I} || I <- lists:seq(1, 1000)],
     Bins = [{Name, make_pid_bin(Name, I, I, I)} || {Name, I} <- Inputs],
     %% Warm up before sampling the atom count.
     {_, B0} = hd(Bins),
-    {ok, _} = rabbit_pid_codec:decompose_from_binary_safe(B0),
+    {ok, _} = rabbit_pid_codec:decompose_from_binary(B0),
     Before = erlang:system_info(atom_count),
     lists:foreach(
       fun({Name, B}) ->
               {ok, #{node := Got}} =
-                  rabbit_pid_codec:decompose_from_binary_safe(B),
+                  rabbit_pid_codec:decompose_from_binary(B),
               ?assertEqual(Name, Got)
       end,
       Bins),
@@ -91,7 +91,7 @@ safe_decoder_handles_long_node_names(_) ->
     Long = binary:copy(<<"a">>, 65535),
     Bin = make_pid_bin(Long, 1, 2, 3),
     {ok, #{node := Got, id := 1, serial := 2, creation := 3}} =
-        rabbit_pid_codec:decompose_from_binary_safe(Bin),
+        rabbit_pid_codec:decompose_from_binary(Bin),
     ?assertEqual(Long, Got).
 
 pid_from_name_no_atoms_for_crafted_input(_) ->

deps/rabbit/test/pid_codec_prop_SUITE.erl

@@ -41,7 +41,7 @@ prop_safe_decoder_returns_ok_or_error(_) ->
 
 prop_safe_decoder_returns_ok_or_error_() ->
     ?FORALL(Bin, binary(),
-            case rabbit_pid_codec:decompose_from_binary_safe(Bin) of
+            case rabbit_pid_codec:decompose_from_binary(Bin) of
                 error -> true;
                 {ok, #{node := N, id := I, serial := S, creation := C}} ->
                     is_binary(N)
@@ -63,7 +63,7 @@ prop_safe_decoder_accepts_arbitrary_node_bytes_() ->
                 Bin = make_pid_bin(NodeBytes, ID, Serial, Creation),
                 {ok, #{node := Got, id := GotID,
                        serial := GotS, creation := GotC}} =
-                    rabbit_pid_codec:decompose_from_binary_safe(Bin),
+                    rabbit_pid_codec:decompose_from_binary(Bin),
                 Got =:= NodeBytes
                     andalso GotID =:= ID
                     andalso GotS =:= Serial
@@ -85,7 +85,7 @@ prop_safe_decoder_roundtrip_numeric_fields_() ->
                 Bin = rabbit_pid_codec:recompose_to_binary(Parts),
                 {ok, #{node := NB, id := GotID,
                        serial := GotS, creation := GotC}} =
-                    rabbit_pid_codec:decompose_from_binary_safe(Bin),
+                    rabbit_pid_codec:decompose_from_binary(Bin),
                 NB =:= NodeBin
                     andalso GotID =:= ID
                     andalso GotS =:= Serial
@@ -94,15 +94,15 @@ prop_safe_decoder_roundtrip_numeric_fields_() ->
 
 prop_safe_decoder_no_atom_growth(_) ->
     %% Warm up before sampling the atom count.
-    _ = rabbit_pid_codec:decompose_from_binary_safe(<<>>),
+    _ = rabbit_pid_codec:decompose_from_binary(<<>>),
     rabbit_ct_proper_helpers:run_proper(
       fun prop_safe_decoder_no_atom_growth_/0, [], ?PROP_ITERATIONS).
 
 prop_safe_decoder_no_atom_growth_() ->
     ?FORALL(Bin, binary(),
             begin
                 Before = erlang:system_info(atom_count),
-                _ = rabbit_pid_codec:decompose_from_binary_safe(Bin),
+                _ = rabbit_pid_codec:decompose_from_binary(Bin),
                 erlang:system_info(atom_count) =:= Before
             end).