From: matz@... (Yukihiro Matsumoto)
Date: 2002-09-13T09:31:26+09:00
Subject: Re: [Add-to-fix] String.c

Hi,

In message "[Add-to-fix] String.c"
    on 02/09/12, Michal Rokos <m.rokos@sh.cvut.cz> writes:

|	matz, thanks for reworking my patch. I'm really sad that you've
|	been forced to work it on yourself.
|
|	Sorry.

Not at all.  It's my joy to hack the code.  I just envied you.

|	I'm OK with your changes. I have just small addon to it.

Thank you.

|@@ -68,11 +68,11 @@ str_new(klass, ptr, len)
|     RSTRING(str)->ptr = ALLOC_N(char,len+1);
|     if (ptr) {
| 	memcpy(RSTRING(str)->ptr, ptr, len);
|+	RSTRING(str)->ptr[len] = '\0';
|     }
|     else {
|-	MEMZERO(RSTRING(str)->ptr, char, len);
|+	MEMZERO(RSTRING(str)->ptr, char, len+1);
|     }
|-    RSTRING(str)->ptr[len] = '\0';
|     return str;
| }
| 
|Nothing important here.

It is possible to remove sentinel NUL from the string tail in the
future, so I thought sentinel assignment should be separated.

|@@ -222,6 +222,7 @@ rb_str_shared_replace(str, str2)
| 	RSTRING(str)->ptr = 0;
| 	RSTRING(str)->len = 0;
| 	RSTRING(str)->aux.capa = 0;
|+	FL_UNSET(str, ELTS_SHARED|STR_ASSOC);
| 	return;
|     }
|     RSTRING(str)->ptr = RSTRING(str2)->ptr;
|@@ -231,6 +232,7 @@ rb_str_shared_replace(str, str2)
| 	RSTRING(str)->aux.shared = RSTRING(str2)->aux.shared;
|     }
|     else {
|+	FL_UNSET(str, ELTS_SHARED|STR_ASSOC);
| 	RSTRING(str)->aux.capa = RSTRING(str2)->aux.capa;
|     }
|     RSTRING(str2)->ptr = 0;	/* abandon str2 */
|I think that unsetting some flags could be handy.

OK.

|@@ -489,10 +491,18 @@ VALUE
| rb_str_dup_frozen(str)
|     VALUE str;
| {
|+#if 0
|+    /*
|+     * Question:
|+     * What is the str is subseq of share
|+     * (str->len < str->aux.shared->len)
|+     * ?
|+     */
|     if (FL_TEST(str, ELTS_SHARED)) {
| 	OBJ_FREEZE(RSTRING(str)->aux.shared);
| 	return RSTRING(str)->aux.shared;
|     }
|+#endif
|     if (OBJ_FROZEN(str)) return str;
|     str = rb_str_dup(str);
|     OBJ_FREEZE(str);
|Please, see the question...

We don't make shared substring now, do we?

|@@ -632,13 +642,11 @@ VALUE
| rb_str_append(str, str2)
|     VALUE str, str2;
| {
|-    long len;
|-
|-    StringValue(str2);
|     rb_str_modify(str);
|+    StringValue(str2);
|     if (RSTRING(str2)->len > 0) {
| 	if (FL_TEST(str, STR_ASSOC)) {
|-	    len = RSTRING(str)->len+RSTRING(str2)->len;
|+	    long len = RSTRING(str)->len+RSTRING(str2)->len;
| 	    REALLOC_N(RSTRING(str)->ptr, char, len+1);
| 	    memcpy(RSTRING(str)->ptr + RSTRING(str)->len,
| 		   RSTRING(str2)->ptr, RSTRING(str2)->len);
|(1) I think that we could check first whether we can modify str,
|before we will start to do something.

Doesn't matter much, but OK.

|@@ -1636,7 +1644,7 @@ rb_str_replace(str, str2)
| 	}
| 	RSTRING(str)->len = RSTRING(str2)->len;
| 	RSTRING(str)->ptr = RSTRING(str2)->ptr;
|-	FL_SET(str, RBASIC(str2)->flags & (ELTS_SHARED|STR_ASSOC));
|+	FL_SET(str, ELTS_SHARED);
| 	RSTRING(str)->aux.shared = RSTRING(str2)->aux.shared;
|     }
|     else {
|(2) To get glags from str2 is not needed. We tested them already.

OK.

|@@ -1644,7 +1652,7 @@ rb_str_replace(str, str2)
| 	rb_str_resize(str, RSTRING(str2)->len);
| 	memcpy(RSTRING(str)->ptr, RSTRING(str2)->ptr, RSTRING(str2)->len);
| 	if (FL_TEST(str2, STR_ASSOC)) {
|-	    FL_SET(str, RBASIC(str2)->flags & (ELTS_SHARED|STR_ASSOC));
|+	    FL_SET(str, STR_ASSOC);
| 	    RSTRING(str)->aux.shared = RSTRING(str2)->aux.shared;
| 	}
|     }
|ditto (2)

OK.

|@@ -2305,6 +2313,7 @@ rb_str_delete_bang(argc, argv, str)
|     if (argc < 1) {
| 	rb_raise(rb_eArgError, "wrong number of arguments");
|     }
|+    rb_str_modify(str);
|     for (i=0; i<argc; i++) {
| 	VALUE s = argv[i];
| 
|ditto (1)

ditto(1) ;-)

|@@ -2313,7 +2322,6 @@ rb_str_delete_bang(argc, argv, str)
| 	init = 0;
|     }
| 
|-    rb_str_modify(str);
|     s = t = RSTRING(str)->ptr;
|     if (!s || RSTRING(str)->len == 0) return Qnil;
|     send = s + RSTRING(str)->len;
|ditto (1)

OK.

|@@ -2354,6 +2362,7 @@ rb_str_squeeze_bang(argc, argv, str)
|     int init = 1;
|     int i;
| 
|+    rb_str_modify(str);
|     if (argc == 0) {
| 	for (i=0; i<256; i++) {
| 	    squeez[i] = 1;
|ditto (1)

OK.

|@@ -2369,7 +2378,6 @@ rb_str_squeeze_bang(argc, argv, str)
| 	}
|     }
| 
|-    rb_str_modify(str);
|     s = t = RSTRING(str)->ptr;
|     if (!s || RSTRING(str)->len == 0) return Qnil;
|     send = s + RSTRING(str)->len;
|ditto (1)

OK.

Thank you.

							matz.