nobu	2015-06-01 07:32:14 +0900 (Mon, 01 Jun 2015)

  New Revision: 50704

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=50704

  Log:
    tkutil.c: fix memory leak and segfault
    
    * ext/tk/tkutil/tkutil.c (cbsubst_append_inf_key): extract a
      function append a key in subst info to a string.  make result
      strings first and get rid of potential memory leak.
    
    * ext/tk/tkutil/tkutil.c (cbsubst_get_subst_arg): allocate the
      result buffer as a string to fix:
    
      * memory leak when the argument key is not found:
    
          loop {Tk::Event.subst_arg(:a) rescue nil}
    
      * buffer overflow segfault when many arguments:
    
          class T < TkUtil::CallbackSubst
            _setup_subst_table([[?a, ?A, :_a]], [])
            subst_arg(*[:_a]*1000).size
          end

  Modified files:
    trunk/ext/tk/tkutil/tkutil.c
Index: ext/tk/tkutil/tkutil.c
===================================================================
--- ext/tk/tkutil/tkutil.c	(revision 50703)
+++ ext/tk/tkutil/tkutil.c	(revision 50704)
@@ -1324,15 +1324,46 @@ cbsubst_def_attr_aliases(self, tbl) https://github.com/ruby/ruby/blob/trunk/ext/tk/tkutil/tkutil.c#L1324
 }
 
 static VALUE
+cbsubst_append_inf_key(str, inf, idx)
+    VALUE str;
+    const struct cbsubst_info *inf;
+    int idx;
+{
+    const long len = inf->keylen[idx];
+    const long olen = RSTRING_LEN(str);
+    char *buf, *ptr;
+
+    rb_str_modify_expand(str, (len ? len : 1) + 2);
+    buf = RSTRING_PTR(str);
+    ptr = buf + olen;
+
+    *(ptr++) = '%';
+
+    if (len != 0) {
+	/* longname */
+	strncpy(ptr, inf->key[idx], len);
+	ptr += len;
+    }
+    else {
+	/* single char */
+	*(ptr++) = (unsigned char)idx;
+    }
+
+    *(ptr++) = ' ';
+
+    rb_str_set_len(str, ptr - buf);
+
+    return str;
+}
+
+static VALUE
 cbsubst_sym_to_subst(self, sym)
     VALUE self;
     VALUE sym;
 {
     struct cbsubst_info *inf;
     VALUE str;
-    char *buf, *ptr;
     int idx;
-    long len;
     ID id;
     volatile VALUE ret;
 
@@ -1353,27 +1384,7 @@ cbsubst_sym_to_subst(self, sym) https://github.com/ruby/ruby/blob/trunk/ext/tk/tkutil/tkutil.c#L1384
     }
     if (idx >= CBSUBST_TBL_MAX)  return sym;
 
-    ptr = buf = ALLOC_N(char, inf->full_subst_length + 1);
-
-    *(ptr++) = '%';
-
-    if ((len = inf->keylen[idx]) != 0) {
-      /* longname */
-      strncpy(ptr, inf->key[idx], len);
-      ptr += len;
-    } else {
-      /* single char */
-      *(ptr++) = (unsigned char)idx;
-    }
-
-    *(ptr++) = ' ';
-    *(ptr++) = '\0';
-
-    ret = rb_str_new2(buf);
-
-    xfree(buf);
-
-    return ret;
+    return cbsubst_append_inf_key(rb_str_new(0, 0), inf, idx);
 }
 
 static VALUE
@@ -1384,16 +1395,13 @@ cbsubst_get_subst_arg(argc, argv, self) https://github.com/ruby/ruby/blob/trunk/ext/tk/tkutil/tkutil.c#L1395
 {
     struct cbsubst_info *inf;
     VALUE str;
-    char *buf, *ptr;
     int i, idx;
-    long len;
     ID id;
-    volatile VALUE arg_sym, ret;
+    VALUE arg_sym, ret, result;
 
     inf = cbsubst_get_ptr(self);
 
-    ptr = buf = ALLOC_N(char, inf->full_subst_length + 1);
-
+    result = rb_str_new(0, 0);
     for(i = 0; i < argc; i++) {
         switch(TYPE(argv[i])) {
         case T_STRING:
@@ -1425,27 +1433,10 @@ cbsubst_get_subst_arg(argc, argv, self) https://github.com/ruby/ruby/blob/trunk/ext/tk/tkutil/tkutil.c#L1433
             rb_raise(rb_eArgError, "cannot find attribute :%"PRIsVALUE, str);
         }
 
-	*(ptr++) = '%';
-
-	if ((len = inf->keylen[idx]) != 0) {
-	  /* longname */
-	  strncpy(ptr, inf->key[idx], len);
-	  ptr += len;
-	} else {
-	  /* single char */
-	  *(ptr++) = (unsigned char)idx;
-	}
-
-	*(ptr++) = ' ';
+	result = cbsubst_append_inf_key(result, inf, idx);
     }
 
-    *ptr = '\0';
-
-    ret = rb_str_new2(buf);
-
-    xfree(buf);
-
-    return ret;
+    return result;
 }
 
 static VALUE
@@ -1454,8 +1445,8 @@ cbsubst_get_subst_key(self, str) https://github.com/ruby/ruby/blob/trunk/ext/tk/tkutil/tkutil.c#L1445
     VALUE str;
 {
     struct cbsubst_info *inf;
-    volatile VALUE list;
-    volatile VALUE ret;
+    VALUE list;
+    VALUE ret;
     long i, len, keylen;
     int idx;
     char *buf, *ptr;
@@ -1466,7 +1457,8 @@ cbsubst_get_subst_key(self, str) https://github.com/ruby/ruby/blob/trunk/ext/tk/tkutil/tkutil.c#L1457
 
     inf = cbsubst_get_ptr(self);
 
-    ptr = buf = ALLOC_N(char, len + 1);
+    ret = rb_str_new(0, len);
+    ptr = buf = RSTRING_PTR(ret);
 
     for(i = 0; i < len; i++) {
       VALUE keyval = RARRAY_CONST_PTR(list)[i];
@@ -1494,10 +1486,8 @@ cbsubst_get_subst_key(self, str) https://github.com/ruby/ruby/blob/trunk/ext/tk/tkutil/tkutil.c#L1486
 	*(ptr++) = ' ';
       }
     }
-    *ptr = '\0';
 
-    ret = rb_str_new2(buf);
-    xfree(buf);
+    rb_str_set_len(ret, ptr - buf);
     return ret;
 }
 
@@ -1506,45 +1496,26 @@ cbsubst_get_all_subst_keys(self) https://github.com/ruby/ruby/blob/trunk/ext/tk/tkutil/tkutil.c#L1496
     VALUE self;
 {
     struct cbsubst_info *inf;
-    char *buf, *ptr;
     char *keys_buf, *keys_ptr;
     int idx;
-    long len;
-    volatile VALUE ret;
+    VALUE str, keys_str;
 
     inf = cbsubst_get_ptr(self);
 
-    ptr = buf = ALLOC_N(char, inf->full_subst_length + 1);
-    keys_ptr = keys_buf = ALLOC_N(char, CBSUBST_TBL_MAX + 1);
+    str = rb_str_new(0, 0);
+    keys_str = rb_str_new(0, CBSUBST_TBL_MAX);
+    keys_ptr = keys_buf = RSTRING_PTR(keys_str);
 
     for(idx = 0; idx < CBSUBST_TBL_MAX; idx++) {
       if (inf->ivar[idx] == (ID) 0) continue;
 
       *(keys_ptr++) = (unsigned char)idx;
 
-      *(ptr++) = '%';
-
-      if ((len = inf->keylen[idx]) != 0) {
-	/* longname */
-	strncpy(ptr, inf->key[idx], len);
-	ptr += len;
-      } else {
-	/* single char */
-	*(ptr++) = (unsigned char)idx;
-      }
-
-      *(ptr++) = ' ';
+      str = cbsubst_append_inf_key(str, inf, idx);
     }
+    rb_str_set_len(keys_str, keys_ptr - keys_buf);
 
-    *ptr = '\0';
-    *keys_ptr = '\0';
-
-    ret = rb_ary_new3(2, rb_str_new2(keys_buf), rb_str_new2(buf));
-
-    xfree(buf);
-    xfree(keys_buf);
-
-    return ret;
+    return rb_ary_new3(2, keys_str, str);
 }
 
 static VALUE

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/