Merge pull request #41 from apolcyn/ruby_char_ptr_to_slice

update ruby char* to slice

6 files changed, 93 insertions, 62 deletions

diff --git a/src/ruby/ext/grpc/rb_byte_buffer.c b/src/ruby/ext/grpc/rb_byte_buffer.c
index f97890e4a2..0f15f70a28 100644
--- a/src/ruby/ext/grpc/rb_byte_buffer.c
+++ b/src/ruby/ext/grpc/rb_byte_buffer.c
@@ -67,3 +67,10 @@ VALUE grpc_rb_byte_buffer_to_s(grpc_byte_buffer *buffer) {
   }
   return rb_string;
 }
+
+VALUE grpc_rb_slice_to_ruby_string(grpc_slice slice) {
+  if (GRPC_SLICE_START_PTR(slice) == NULL) {
+    rb_raise(rb_eRuntimeError, "attempt to convert uninitialized grpc_slice to ruby string");
+  }
+  return rb_str_new((char*)GRPC_SLICE_START_PTR(slice), GRPC_SLICE_LENGTH(slice));
+}
diff --git a/src/ruby/ext/grpc/rb_byte_buffer.h b/src/ruby/ext/grpc/rb_byte_buffer.h
index c7ddd76489..fac68fe6a0 100644
--- a/src/ruby/ext/grpc/rb_byte_buffer.h
+++ b/src/ruby/ext/grpc/rb_byte_buffer.h
@@ -44,4 +44,7 @@ grpc_byte_buffer *grpc_rb_s_to_byte_buffer(char *string, size_t length);
 /* Converts a grpc_byte_buffer to a ruby string */
 VALUE grpc_rb_byte_buffer_to_s(grpc_byte_buffer *buffer);
 
+/* Converts a grpc_slice to a ruby string */
+VALUE grpc_rb_slice_to_ruby_string(grpc_slice slice);
+
 #endif /* GRPC_RB_BYTE_BUFFER_H_ */
diff --git a/src/ruby/ext/grpc/rb_call.c b/src/ruby/ext/grpc/rb_call.c
index 67a42af619..0179bd9e9b 100644
--- a/src/ruby/ext/grpc/rb_call.c
+++ b/src/ruby/ext/grpc/rb_call.c
@@ -121,8 +121,8 @@ static size_t md_ary_datasize(const void *p) {
   size_t i, datasize = sizeof(grpc_metadata_array);
   for (i = 0; i < ary->count; ++i) {
     const grpc_metadata *const md = &ary->metadata[i];
-    datasize += strlen(md->key);
-    datasize += md->value_length;
+    datasize += GRPC_SLICE_LENGTH(md->key);
+    datasize += GRPC_SLICE_LENGTH(md->value);
   }
   datasize += ary->capacity * sizeof(grpc_metadata);
   return datasize;
@@ -386,23 +386,23 @@ static int grpc_rb_md_ary_fill_hash_cb(VALUE key, VALUE val, VALUE md_ary_obj) {
   grpc_metadata_array *md_ary = NULL;
   long array_length;
   long i;
-  char *key_str;
-  size_t key_len;
-  char *value_str;
-  size_t value_len;
+  grpc_slice key_slice;
+  grpc_slice value_slice;
+  char* tmp_str;
 
   if (TYPE(key) == T_SYMBOL) {
-    key_str = (char *)rb_id2name(SYM2ID(key));
-    key_len = strlen(key_str);
-  } else { /* StringValueCStr does all other type exclusions for us */
-    key_str = StringValueCStr(key);
-    key_len = RSTRING_LEN(key);
+    key_slice = grpc_slice_from_static_string(rb_id2name(SYM2ID(key)));
+  } else if (TYPE(key) == T_STRING) {
+    key_slice = grpc_slice_from_copied_buffer(RSTRING_PTR(key), RSTRING_LEN(key));
+  } else {
+    rb_raise(rb_eTypeError, "grpc_rb_md_ary_fill_hash_cb: bad type for key parameter");
   }
 
-  if (!grpc_header_key_is_legal(key_str, key_len)) {
+  if (!grpc_header_key_is_legal(key_slice)) {
+    tmp_str = grpc_slice_to_c_string(key_slice);
     rb_raise(rb_eArgError,
              "'%s' is an invalid header key, must match [a-z0-9-_.]+",
-             key_str);
+             tmp_str);
     return ST_STOP;
   }
 
@@ -414,33 +414,31 @@ static int grpc_rb_md_ary_fill_hash_cb(VALUE key, VALUE val, VALUE md_ary_obj) {
     array_length = RARRAY_LEN(val);
     /* If the value is an array, add capacity for each value in the array */
     for (i = 0; i < array_length; i++) {
-      value_str = RSTRING_PTR(rb_ary_entry(val, i));
-      value_len = RSTRING_LEN(rb_ary_entry(val, i));
-      if (!grpc_is_binary_header(key_str, key_len) &&
-          !grpc_header_nonbin_value_is_legal(value_str, value_len)) {
+      value_slice = grpc_slice_from_copied_buffer(RSTRING_PTR(rb_ary_entry(val, i)), RSTRING_LEN(rb_ary_entry(val, i)));
+      if (!grpc_is_binary_header(key_slice) &&
+          !grpc_header_nonbin_value_is_legal(value_slice)) {
         // The value has invalid characters
+        tmp_str = grpc_slice_to_c_string(value_slice);
         rb_raise(rb_eArgError,
-                 "Header value '%s' has invalid characters", value_str);
+                 "Header value '%s' has invalid characters", tmp_str);
         return ST_STOP;
       }
-      md_ary->metadata[md_ary->count].key = key_str;
-      md_ary->metadata[md_ary->count].value = value_str;
-      md_ary->metadata[md_ary->count].value_length = value_len;
+      md_ary->metadata[md_ary->count].key = key_slice;
+      md_ary->metadata[md_ary->count].value = value_slice;
       md_ary->count += 1;
     }
   } else if (TYPE(val) == T_STRING) {
-    value_str = RSTRING_PTR(val);
-    value_len = RSTRING_LEN(val);
-    if (!grpc_is_binary_header(key_str, key_len) &&
-        !grpc_header_nonbin_value_is_legal(value_str, value_len)) {
+    value_slice = grpc_slice_from_copied_buffer(RSTRING_PTR(val), RSTRING_LEN(val));
+    if (!grpc_is_binary_header(key_slice) &&
+        !grpc_header_nonbin_value_is_legal(value_slice)) {
       // The value has invalid characters
+      tmp_str = grpc_slice_to_c_string(value_slice);
       rb_raise(rb_eArgError,
-               "Header value '%s' has invalid characters", value_str);
+               "Header value '%s' has invalid characters", tmp_str);
       return ST_STOP;
     }
-    md_ary->metadata[md_ary->count].key = key_str;
-    md_ary->metadata[md_ary->count].value = value_str;
-    md_ary->metadata[md_ary->count].value_length = value_len;
+    md_ary->metadata[md_ary->count].key = key_slice;
+    md_ary->metadata[md_ary->count].value = value_slice;
     md_ary->count += 1;
   } else {
     rb_raise(rb_eArgError,
@@ -506,22 +504,19 @@ VALUE grpc_rb_md_ary_to_h(grpc_metadata_array *md_ary) {
   size_t i;
 
   for (i = 0; i < md_ary->count; i++) {
-    key = rb_str_new2(md_ary->metadata[i].key);
+    key = grpc_rb_slice_to_ruby_string(md_ary->metadata[i].key);
     value = rb_hash_aref(result, key);
     if (value == Qnil) {
-      value = rb_str_new(md_ary->metadata[i].value,
-                         md_ary->metadata[i].value_length);
+      value = grpc_rb_slice_to_ruby_string(md_ary->metadata[i].value);
       rb_hash_aset(result, key, value);
     } else if (TYPE(value) == T_ARRAY) {
       /* Add the string to the returned array */
-      rb_ary_push(value, rb_str_new(md_ary->metadata[i].value,
-                                    md_ary->metadata[i].value_length));
+      rb_ary_push(value, grpc_rb_slice_to_ruby_string(md_ary->metadata[i].value));
     } else {
       /* Add the current value with this key and the new one to an array */
       new_ary = rb_ary_new();
       rb_ary_push(new_ary, value);
-      rb_ary_push(new_ary, rb_str_new(md_ary->metadata[i].value,
-                                      md_ary->metadata[i].value_length));
+      rb_ary_push(new_ary, grpc_rb_slice_to_ruby_string(md_ary->metadata[i].value));
       rb_hash_aset(result, key, new_ary);
     }
   }
@@ -563,6 +558,7 @@ static int grpc_rb_call_check_op_keys_hash_cb(VALUE key, VALUE val,
 */
 static void grpc_rb_op_update_status_from_server(grpc_op *op,
                                                  grpc_metadata_array *md_ary,
+                                                 grpc_slice *send_status_details,
                                                  VALUE status) {
   VALUE code = rb_struct_aref(status, sym_code);
   VALUE details = rb_struct_aref(status, sym_details);
@@ -579,8 +575,11 @@ static void grpc_rb_op_update_status_from_server(grpc_op *op,
              rb_obj_classname(code));
     return;
   }
+
+  *send_status_details = grpc_slice_from_copied_buffer(RSTRING_PTR(details), RSTRING_LEN(details));
+
   op->data.send_status_from_server.status = NUM2INT(code);
-  op->data.send_status_from_server.status_details = StringValueCStr(details);
+  op->data.send_status_from_server.status_details = send_status_details;
   grpc_rb_md_ary_convert(metadata_hash, md_ary);
   op->data.send_status_from_server.trailing_metadata_count = md_ary->count;
   op->data.send_status_from_server.trailing_metadata = md_ary->metadata;
@@ -603,9 +602,9 @@ typedef struct run_batch_stack {
   grpc_metadata_array recv_trailing_metadata;
   int recv_cancelled;
   grpc_status_code recv_status;
-  char *recv_status_details;
-  size_t recv_status_details_capacity;
+  grpc_slice recv_status_details;
   unsigned write_flag;
+  grpc_slice send_status_details;
 } run_batch_stack;
 
 /* grpc_run_batch_stack_init ensures the run_batch_stack is properly
@@ -631,8 +630,12 @@ static void grpc_run_batch_stack_cleanup(run_batch_stack *st) {
   grpc_metadata_array_destroy(&st->recv_metadata);
   grpc_metadata_array_destroy(&st->recv_trailing_metadata);
 
-  if (st->recv_status_details != NULL) {
-    gpr_free(st->recv_status_details);
+  if (GRPC_SLICE_START_PTR(st->send_status_details) != NULL) {
+    grpc_slice_unref(st->send_status_details);
+  }
+
+  if (GRPC_SLICE_START_PTR(st->recv_status_details) != NULL) {
+    grpc_slice_unref(st->recv_status_details);
   }
 
   if (st->recv_message != NULL) {
@@ -683,7 +686,7 @@ static void grpc_run_batch_stack_fill_ops(run_batch_stack *st, VALUE ops_hash) {
         /* N.B. later there is no need to explicitly delete the metadata keys
          * and values, they are references to data in ruby objects. */
         grpc_rb_op_update_status_from_server(
-            &st->ops[st->op_num], &st->send_trailing_metadata, this_value);
+            &st->ops[st->op_num], &st->send_trailing_metadata, &st->send_status_details, this_value);
         break;
       case GRPC_OP_RECV_INITIAL_METADATA:
         st->ops[st->op_num].data.recv_initial_metadata = &st->recv_metadata;
@@ -698,8 +701,6 @@ static void grpc_run_batch_stack_fill_ops(run_batch_stack *st, VALUE ops_hash) {
             &st->recv_status;
         st->ops[st->op_num].data.recv_status_on_client.status_details =
             &st->recv_status_details;
-        st->ops[st->op_num].data.recv_status_on_client.status_details_capacity =
-            &st->recv_status_details_capacity;
         break;
       case GRPC_OP_RECV_CLOSE_ON_SERVER:
         st->ops[st->op_num].data.recv_close_on_server.cancelled =
@@ -747,9 +748,9 @@ static VALUE grpc_run_batch_stack_build_result(run_batch_stack *st) {
         rb_struct_aset(
             result, sym_status,
             rb_struct_new(grpc_rb_sStatus, UINT2NUM(st->recv_status),
-                          (st->recv_status_details == NULL
+                          (GRPC_SLICE_START_PTR(st->recv_status_details) == NULL
                                ? Qnil
-                               : rb_str_new2(st->recv_status_details)),
+                               : grpc_rb_slice_to_ruby_string(st->recv_status_details)),
                           grpc_rb_md_ary_to_h(&st->recv_trailing_metadata),
                           NULL));
         break;
diff --git a/src/ruby/ext/grpc/rb_channel.c b/src/ruby/ext/grpc/rb_channel.c
index 3b2b88eb77..84e43d3f7b 100644
--- a/src/ruby/ext/grpc/rb_channel.c
+++ b/src/ruby/ext/grpc/rb_channel.c
@@ -35,6 +35,7 @@
 
 #include "rb_grpc_imports.generated.h"
 #include "rb_channel.h"
+#include "rb_byte_buffer.h"
 
 #include <grpc/grpc.h>
 #include <grpc/grpc_security.h>
@@ -252,10 +253,14 @@ static VALUE grpc_rb_channel_create_call(VALUE self, VALUE parent,
   grpc_channel *ch = NULL;
   grpc_completion_queue *cq = NULL;
   int flags = GRPC_PROPAGATE_DEFAULTS;
-  char *method_chars = StringValueCStr(method);
-  char *host_chars = NULL;
+  grpc_slice method_slice;
+  grpc_slice host_slice;
+  grpc_slice *host_slice_ptr = NULL;
+  char* tmp_str = NULL;
+
   if (host != Qnil) {
-    host_chars = StringValueCStr(host);
+    host_slice = grpc_slice_from_copied_buffer(RSTRING_PTR(host), RSTRING_LEN(host));
+    host_slice_ptr = &host_slice;
   }
   if (mask != Qnil) {
     flags = NUM2UINT(mask);
@@ -272,15 +277,25 @@ static VALUE grpc_rb_channel_create_call(VALUE self, VALUE parent,
     return Qnil;
   }
 
-  call = grpc_channel_create_call(ch, parent_call, flags, cq, method_chars,
-                                  host_chars, grpc_rb_time_timeval(
+  method_slice = grpc_slice_from_copied_buffer(RSTRING_PTR(method), RSTRING_LEN(method));
+
+  call = grpc_channel_create_call(ch, parent_call, flags, cq, method_slice,
+                                  host_slice_ptr, grpc_rb_time_timeval(
                                       deadline,
                                       /* absolute time */ 0), NULL);
+
   if (call == NULL) {
+    tmp_str = grpc_slice_to_c_string(method_slice);
     rb_raise(rb_eRuntimeError, "cannot create call with method %s",
-             method_chars);
+             tmp_str);
     return Qnil;
   }
+
+  grpc_slice_unref(method_slice);
+  if (host_slice_ptr != NULL) {
+    grpc_slice_unref(host_slice);
+  }
+
   res = grpc_rb_wrap_call(call, cq);
 
   /* Make this channel an instance attribute of the call so that it is not GCed
diff --git a/src/ruby/ext/grpc/rb_compression_options.c b/src/ruby/ext/grpc/rb_compression_options.c
index 6200dbafeb..6b2467ee46 100644
--- a/src/ruby/ext/grpc/rb_compression_options.c
+++ b/src/ruby/ext/grpc/rb_compression_options.c
@@ -34,6 +34,7 @@
 #include <ruby/ruby.h>
 
 #include "rb_compression_options.h"
+#include "rb_byte_buffer.h"
 #include "rb_grpc_imports.generated.h"
 
 #include <grpc/compression.h>
@@ -168,9 +169,9 @@ void grpc_rb_compression_options_set_default_level(
  * Raises an error if the name of the algorithm passed in is invalid. */
 void grpc_rb_compression_options_algorithm_name_to_value_internal(
     grpc_compression_algorithm *algorithm_value, VALUE algorithm_name) {
-  char *name_str = NULL;
-  long name_len = 0;
+  grpc_slice name_slice;
   VALUE algorithm_name_as_string = Qnil;
+  char *tmp_str = NULL;
 
   Check_Type(algorithm_name, T_SYMBOL);
 
@@ -178,16 +179,18 @@ void grpc_rb_compression_options_algorithm_name_to_value_internal(
    * correct C string out of it. */
   algorithm_name_as_string = rb_funcall(algorithm_name, rb_intern("to_s"), 0);
 
-  name_str = RSTRING_PTR(algorithm_name_as_string);
-  name_len = RSTRING_LEN(algorithm_name_as_string);
+  name_slice = grpc_slice_from_copied_buffer(RSTRING_PTR(algorithm_name_as_string), RSTRING_LEN(algorithm_name_as_string));
 
   /* Raise an error if the name isn't recognized as a compression algorithm by
    * the algorithm parse function
    * in GRPC core. */
-  if (!grpc_compression_algorithm_parse(name_str, name_len, algorithm_value)) {
+  if(!grpc_compression_algorithm_parse(name_slice, algorithm_value)) {
+    tmp_str = grpc_slice_to_c_string(name_slice);
     rb_raise(rb_eNameError, "Invalid compression algorithm name: %s",
-             StringValueCStr(algorithm_name_as_string));
+             tmp_str);
   }
+
+  grpc_slice_unref(name_slice);
 }
 
 /* Indicates whether a given algorithm is enabled on this instance, given the
diff --git a/src/ruby/ext/grpc/rb_server.c b/src/ruby/ext/grpc/rb_server.c
index 2a6a246e67..7caafe7b8d 100644
--- a/src/ruby/ext/grpc/rb_server.c
+++ b/src/ruby/ext/grpc/rb_server.c
@@ -43,6 +43,7 @@
 #include "rb_channel_args.h"
 #include "rb_completion_queue.h"
 #include "rb_server_credentials.h"
+#include "rb_byte_buffer.h"
 #include "rb_grpc.h"
 
 /* grpc_rb_cServer is the ruby class that proxies grpc_server. */
@@ -160,8 +161,6 @@ static void grpc_request_call_stack_init(request_call_stack* st) {
   MEMZERO(st, request_call_stack, 1);
   grpc_metadata_array_init(&st->md_ary);
   grpc_call_details_init(&st->details);
-  st->details.method = NULL;
-  st->details.host = NULL;
 }
 
 /* grpc_request_call_stack_cleanup ensures the request_call_stack is properly
@@ -185,6 +184,7 @@ static VALUE grpc_rb_server_request_call(VALUE self) {
   void *tag = (void*)&st;
   grpc_completion_queue *call_queue = grpc_completion_queue_create(NULL);
   gpr_timespec deadline;
+
   TypedData_Get_Struct(self, grpc_rb_server, &grpc_rb_server_data_type, s);
   if (s->wrapped == NULL) {
     rb_raise(rb_eRuntimeError, "destroyed!");
@@ -212,11 +212,13 @@ static VALUE grpc_rb_server_request_call(VALUE self) {
     return Qnil;
   }
 
+
+
   /* build the NewServerRpc struct result */
   deadline = gpr_convert_clock_type(st.details.deadline, GPR_CLOCK_REALTIME);
   result = rb_struct_new(
-      grpc_rb_sNewServerRpc, rb_str_new2(st.details.method),
-      rb_str_new2(st.details.host),
+      grpc_rb_sNewServerRpc, grpc_rb_slice_to_ruby_string(st.details.method),
+      grpc_rb_slice_to_ruby_string(st.details.host),
       rb_funcall(rb_cTime, id_at, 2, INT2NUM(deadline.tv_sec),
                  INT2NUM(deadline.tv_nsec / 1000)),
       grpc_rb_md_ary_to_h(&st.md_ary), grpc_rb_wrap_call(call, call_queue),