kpumuk commented on code in PR #3703:
URL: https://github.com/apache/thrift/pull/3703#discussion_r3741275205


##########
lib/rb/ext/compact_protocol.c:
##########
@@ -243,13 +279,15 @@ VALUE rb_thrift_compact_proto_write_field_stop(VALUE 
self) {
 }
 
 VALUE rb_thrift_compact_proto_write_map_begin(VALUE self, VALUE ktype, VALUE 
vtype, VALUE size_value) {
-  int size = FIX2INT(size_value);
+  int size = checked_size_value(size_value);
+  int key_type = get_compact_type(ktype);
+  int value_type = get_compact_type(vtype);
   VALUE transport = GET_TRANSPORT(self);

Review Comment:
   Accepted



##########
lib/rb/lib/thrift/protocol/compact_protocol.rb:
##########
@@ -155,32 +175,35 @@ def write_field_begin_internal(type, id, type_override = 
nil)
       last_id = @last_field.pop
 
       # if there's a type override, use that.
-      typeToWrite = type_override || CompactTypes.get_compact_type(type)
+      type_to_write = type_override || CompactTypes.get_compact_type(type)
 
       # check if we can use delta encoding for the field id
       if id > last_id && id - last_id <= 15
         # write them together
-        write_byte((id - last_id) << 4 | typeToWrite)
+        write_byte_direct((id - last_id) << 4 | type_to_write)
       else
         # write them separate
-        write_byte(typeToWrite)
-        write_i16(id)
+        write_byte_direct(type_to_write)
+        write_varint32(int_to_zig_zag(id))
       end
 
       @last_field.push(id)
       nil
     end
 
     def write_field_stop
-      write_byte(Types::STOP)
+      write_byte_direct(Types::STOP)
     end
 
     def write_map_begin(ktype, vtype, size)
+      size = validate_size(size)
+      key_type = CompactTypes.get_compact_type(ktype)
+      value_type = CompactTypes.get_compact_type(vtype)
       if (size == 0)

Review Comment:
   Accepted



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to