Re: [Add-to-fix] String.c
From:
matz@... (Yukihiro Matsumoto)
Date:
2002-09-13 00:31:26 UTC
List:
ruby-core #484
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.