Merge pull request #2389 from ksss/str-mem-leaks

Fix memory leaks in String

mrbgems/mruby-string-ext/mrblib/string.rb

@@ -1,4 +1,18 @@
 class String
+
+  ##
+  # call-seq:
+  #    string.clear    ->  string
+  #
+  # Makes string empty.
+  #
+  #    a = "abcde"
+  #    a.clear    #=> ""
+  #
+  def clear
+    self.replace("")
+  end
+
   ##
   # call-seq:
   #    str.lstrip   -> new_str

mrbgems/mruby-string-ext/src/string.c

@@ -239,37 +239,6 @@ mrb_str_lines(mrb_state *mrb, mrb_value self)
   return result;
 }
 
-/*
- *  call-seq:
- *     string.clear    ->  string
- *
- *  Makes string empty.
- *
- *     a = "abcde"
- *     a.clear    #=> ""
- */
-static mrb_value
-mrb_str_clear(mrb_state *mrb, mrb_value str)
-{
-  struct RString *s = mrb_str_ptr(str);
-
-  if (!RSTR_SHARED_P(s) && !RSTR_EMBED_P(s)) {
-    if (RSTR_NOFREE_P(s)) {
-      RSTR_UNSET_NOFREE_FLAG(s);
-    }
-    else {
-      mrb_free(mrb, s->as.heap.ptr);
-    }
-    s->as.heap.ptr = 0;
-    s->as.heap.len = 0;
-  }
-  RSTR_UNSET_SHARED_FLAG(s);
-  RSTR_SET_EMBED_FLAG(s);
-  RSTR_SET_EMBED_LEN(s, 0);
-  RSTRING_PTR(str)[0] = '\0';
-  return str;
-}
-
 void
 mrb_mruby_string_ext_gem_init(mrb_state* mrb)
 {
@@ -287,7 +256,6 @@ mrb_mruby_string_ext_gem_init(mrb_state* mrb)
   mrb_define_method(mrb, s, "oct",             mrb_str_oct,             MRB_ARGS_NONE());
   mrb_define_method(mrb, s, "chr",             mrb_str_chr,             MRB_ARGS_NONE());
   mrb_define_method(mrb, s, "lines",           mrb_str_lines,           MRB_ARGS_NONE());
-  mrb_define_method(mrb, s, "clear",           mrb_str_clear,           MRB_ARGS_NONE());
 }
 
 void

src/string.c

@@ -1368,14 +1368,17 @@ str_replace(mrb_state *mrb, struct RString *s1, struct RString *s2)
   long len;
 
   len = RSTR_LEN(s2);
+  if (RSTR_SHARED_P(s1)) {
+    str_decref(mrb, s1->as.heap.aux.shared);
+  }
+  else if (!RSTR_EMBED_P(s1) && !RSTR_NOFREE_P(s1)) {
+    mrb_free(mrb, s1->as.heap.ptr);
+  }
+
+  RSTR_UNSET_NOFREE_FLAG(s1);
+
   if (RSTR_SHARED_P(s2)) {
-  L_SHARE:
-    if (RSTR_SHARED_P(s1)) {
-      str_decref(mrb, s1->as.heap.aux.shared);
-    }
-    else if (!RSTR_EMBED_P(s1) && !RSTR_NOFREE_P(s1)) {
-      mrb_free(mrb, s1->as.heap.ptr);
-    }
+L_SHARE:
     RSTR_UNSET_EMBED_FLAG(s1);
     s1->as.heap.ptr = s2->as.heap.ptr;
     s1->as.heap.len = len;
@@ -1385,6 +1388,7 @@ str_replace(mrb_state *mrb, struct RString *s1, struct RString *s2)
   }
   else {
     if (len <= RSTRING_EMBED_LEN_MAX) {
+      RSTR_UNSET_SHARED_FLAG(s1);
       RSTR_SET_EMBED_FLAG(s1);
       memcpy(s1->as.ary, RSTR_PTR(s2), len);
       RSTR_SET_EMBED_LEN(s1, len);

test/t/string.rb

@@ -320,6 +320,13 @@ assert('String#replace', '15.2.10.5.28') do
   b.replace(c);
   c.replace(b);
   assert_equal c, b
+
+  # shared string
+  s = "foo" * 100
+  a = s[10, 90]                # create shared string
+  assert_equal("", s.replace(""))    # clear
+  assert_equal("", s)          # s is cleared
+  assert_not_equal("", a)      # a should not be affected
 end
 
 assert('String#reverse', '15.2.10.5.29') do