From: Michal Rokos Date: 2002-09-12T01:28:25+09:00 Subject: Re: [Cleanup, fixes] string.c Hello, On Wed, Sep 11, 2002 at 12:23:09AM +0900, Yukihiro Matsumoto wrote: > Your changes seems OK. But I want functions to be static as much as > possible, so rb_str_modify_check() and str_independent() are better to > be static. In addition, I miss a few modifies you dropped. I will do > it for you. I don't want to waste your time with reworking my patches, so I made new one. (I'm perfectly OK when you say: not like this, do this. I'm definitely more happy when you can work on some more important things!) Michal PS: Some comments inside. Index: string.c =================================================================== RCS file: /src/ruby/string.c,v retrieving revision 1.118 diff -u -p -r1.118 string.c --- string.c 2002/09/11 04:05:36 1.118 +++ string.c 2002/09/11 16:22:13 @@ -27,11 +27,12 @@ VALUE rb_cString; -#define STR_ASSOC FL_USER3 +#define STR_ASSOC FL_USER3 #define RESIZE_CAPA(str,capacity) do {\ REALLOC_N(RSTRING(str)->ptr, char, (capacity)+1);\ RSTRING(str)->aux.capa = (capacity);\ + FL_UNSET(str, STR_ASSOC);\ } while (0) VALUE rb_fs; @@ -61,18 +62,18 @@ str_new(klass, ptr, len) if (len < 0) { rb_raise(rb_eArgError, "negative string size (or size too big)"); } - str = rb_obj_alloc(klass); RSTRING(str)->len = len; RSTRING(str)->aux.capa = 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; } @@ -212,27 +213,62 @@ rb_str_to_str(str) return rb_convert_type(str, T_STRING, "String", "to_str"); } +static int +str_check_independent(str) + VALUE str; +{ + if (OBJ_FROZEN(str)) rb_error_frozen("string"); + if (!OBJ_TAINTED(str) && rb_safe_level() >= 4) { + rb_raise(rb_eSecurityError, "Insecure: can't modify string"); + } + if (!FL_TEST(str, ELTS_SHARED)) return 1; + return 0; +} + static void +str_make_independent(str) + VALUE str; +{ + char *ptr; + long len = RSTRING(str)->len; + + ptr = ALLOC_N(char, len+1); + if (RSTRING(str)->ptr) { + memcpy(ptr, RSTRING(str)->ptr, len); + ptr[len] = '\0'; + } + else { + MEMZERO(ptr, char, len+1); + } + RSTRING(str)->ptr = ptr; + RSTRING(str)->aux.capa = len; + FL_UNSET(str, ELTS_SHARED|STR_ASSOC); +} + +void +rb_str_modify(str) + VALUE str; +{ + if (!str_check_independent(str)) { + str_make_independent(str); + } +} + +static void rb_str_shared_replace(str, str2) VALUE str, str2; { if (str == str2) return; - if (!FL_TEST(str, ELTS_SHARED)) free(RSTRING(str)->ptr); - if (NIL_P(str2)) { - RSTRING(str)->ptr = 0; - RSTRING(str)->len = 0; - RSTRING(str)->aux.capa = 0; - return; + if (str_check_independent(str)) free(RSTRING(str)->ptr); + +#if 0 + if (NIL_P(str2) || FL_TEST(str2, ELTS_SHARED|STR_ASSOC)) { + rb_bug("rb_str_shared_replace: Str2 is not PURE String, as matz said!"); } +#endif RSTRING(str)->ptr = RSTRING(str2)->ptr; RSTRING(str)->len = RSTRING(str2)->len; - if (FL_TEST(str2, ELTS_SHARED|STR_ASSOC)) { - FL_SET(str, RBASIC(str2)->flags & (ELTS_SHARED|STR_ASSOC)); - RSTRING(str)->aux.shared = RSTRING(str2)->aux.shared; - } - else { - RSTRING(str)->aux.capa = RSTRING(str2)->aux.capa; - } + RSTRING(str)->aux.capa = RSTRING(str2)->aux.capa; RSTRING(str2)->ptr = 0; /* abandon str2 */ RSTRING(str2)->len = 0; RSTRING(str2)->aux.capa = 0; @@ -250,16 +286,12 @@ rb_str_associate(str, add) } else { if (FL_TEST(str, ELTS_SHARED)) { - rb_str_modify(str); + str_make_independent(str); } - else if (RSTRING(str)->aux.shared) { - /* str_buf */ - if (RSTRING(str)->aux.capa != RSTRING(str)->len) { - RESIZE_CAPA(str, RSTRING(str)->len); - } + if (RSTRING(str)->aux.capa != RSTRING(str)->len) { + RESIZE_CAPA(str, RSTRING(str)->len); } RSTRING(str)->aux.shared = add; - FL_UNSET(str, ELTS_SHARED); FL_SET(str, STR_ASSOC); } } @@ -398,41 +430,6 @@ rb_str_format(str, arg) return rb_f_sprintf(2, argv); } -static int -str_independent(str) - VALUE str; -{ - if (OBJ_FROZEN(str)) rb_error_frozen("string"); - if (!OBJ_TAINTED(str) && rb_safe_level() >= 4) - rb_raise(rb_eSecurityError, "Insecure: can't modify string"); - if (!FL_TEST(str, ELTS_SHARED)) return 1; - return 0; -} - -static void -str_make_independent(str) - VALUE str; -{ - char *ptr; - - ptr = ALLOC_N(char, RSTRING(str)->len+1); - if (RSTRING(str)->ptr) { - memcpy(ptr, RSTRING(str)->ptr, RSTRING(str)->len); - } - ptr[RSTRING(str)->len] = 0; - RSTRING(str)->ptr = ptr; - RSTRING(str)->aux.capa = RSTRING(str)->len; - FL_UNSET(str, ELTS_SHARED|STR_ASSOC); -} - -void -rb_str_modify(str) - VALUE str; -{ - if (!str_independent(str)) - str_make_independent(str); -} - VALUE rb_string_value(ptr) volatile VALUE *ptr; @@ -493,10 +490,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); @@ -511,15 +516,14 @@ rb_str_resize(str, len) if (len < 0) { rb_raise(rb_eArgError, "negative string size (or size too big)"); } - if (len != RSTRING(str)->len) { rb_str_modify(str); if (RSTRING(str)->len < len || RSTRING(str)->len - len > 1024) { RESIZE_CAPA(str, len); } - RSTRING(str)->len = len; RSTRING(str)->ptr[len] = '\0'; /* sentinel */ + RSTRING(str)->len = len; } return str; } @@ -532,10 +536,20 @@ rb_str_buf_cat(str, ptr, len) { long capa, total; + if (len < 0) { + rb_raise(rb_eArgError, "negative string size (or size too big)"); + } + if (len == 0) return str; if (FL_TEST(str, ELTS_SHARED)) { - rb_str_modify(str); + str_make_independent(str); + } + if (FL_TEST(str, STR_ASSOC)) { + FL_UNSET(str, STR_ASSOC); + capa = RSTRING(str)->len; } - capa = RSTRING(str)->aux.capa; + else { + capa = RSTRING(str)->aux.capa; + } total = RSTRING(str)->len+len; if (capa <= total) { while (total > capa) { @@ -544,8 +558,8 @@ rb_str_buf_cat(str, ptr, len) RESIZE_CAPA(str, capa); } memcpy(RSTRING(str)->ptr + RSTRING(str)->len, ptr, len); - RSTRING(str)->len = total; RSTRING(str)->ptr[total] = '\0'; /* sentinel */ + RSTRING(str)->len = total; return str; } @@ -565,22 +579,7 @@ rb_str_cat(str, ptr, len) long len; { rb_str_modify(str); - if (len > 0) { - if (!FL_TEST(str, ELTS_SHARED) && !FL_TEST(str, STR_ASSOC)) { - return rb_str_buf_cat(str, ptr, len); - } - RESIZE_CAPA(str, RSTRING(str)->len + len); - if (ptr) { - memcpy(RSTRING(str)->ptr + RSTRING(str)->len, ptr, len); - } - else { - MEMZERO(RSTRING(str)->ptr + RSTRING(str)->len, char, len); - } - RSTRING(str)->len += len; - RSTRING(str)->ptr[RSTRING(str)->len] = '\0'; /* sentinel */ - } - - return str; + return rb_str_buf_cat(str, ptr, len); } VALUE @@ -598,11 +597,17 @@ rb_str_buf_append(str, str2) long capa, len; if (FL_TEST(str, ELTS_SHARED)) { - rb_str_modify(str); + str_make_independent(str); } - capa = RSTRING(str)->aux.capa; - + if (FL_TEST(str, STR_ASSOC)) { + FL_UNSET(str, STR_ASSOC); + capa = RSTRING(str)->len; + } + else { + capa = RSTRING(str)->aux.capa; + } len = RSTRING(str)->len+RSTRING(str2)->len; + if (capa <= len) { while (len > capa) { capa = (capa + 1) * 2; @@ -621,25 +626,12 @@ VALUE rb_str_append(str, str2) VALUE str, str2; { - long len; - - StringValue(str2); rb_str_modify(str); - if (RSTRING(str2)->len > 0) { - len = RSTRING(str)->len+RSTRING(str2)->len; - if (!FL_TEST(str, ELTS_SHARED) && !FL_TEST(str, STR_ASSOC)) { - rb_str_buf_append(str, str2); - OBJ_INFECT(str, str2); - return str; - } - RESIZE_CAPA(str, len); - memcpy(RSTRING(str)->ptr + RSTRING(str)->len, - RSTRING(str2)->ptr, RSTRING(str2)->len); - RSTRING(str)->len += RSTRING(str2)->len; - RSTRING(str)->ptr[RSTRING(str)->len] = '\0'; /* sentinel */ - } - OBJ_INFECT(str, str2); + StringValue(str2); + rb_str_buf_append(str, str2); + + OBJ_INFECT(str, str2); return str; } @@ -1575,7 +1567,7 @@ str_gsub(argc, argv, str, bang) } rb_backref_set(match); if (bang) { - if (str_independent(str)) { + if (!FL_TEST(str, ELTS_SHARED)) { free(RSTRING(str)->ptr); } FL_UNSET(str, ELTS_SHARED|STR_ASSOC); @@ -1601,6 +1593,7 @@ rb_str_gsub_bang(argc, argv, str) VALUE *argv; VALUE str; { + str_check_independent(str); return str_gsub(argc, argv, str, 1); } @@ -1619,23 +1612,27 @@ rb_str_replace(str, str2) { if (str == str2) return str; + str_check_independent(str); StringValue(str2); + if (FL_TEST(str2, ELTS_SHARED)) { - if (str_independent(str)) { + if (!FL_TEST(str, ELTS_SHARED)) { free(RSTRING(str)->ptr); } RSTRING(str)->len = RSTRING(str2)->len; RSTRING(str)->ptr = RSTRING(str2)->ptr; - FL_SET(str, RBASIC(str2)->flags & (ELTS_SHARED|STR_ASSOC)); RSTRING(str)->aux.shared = RSTRING(str2)->aux.shared; + FL_SET(str, ELTS_SHARED); } else { - rb_str_modify(str); + if (FL_TEST(str, ELTS_SHARED)) { + str_make_independent(str); + } 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)); RSTRING(str)->aux.shared = RSTRING(str2)->aux.shared; + FL_SET(str, STR_ASSOC); } } @@ -2295,6 +2292,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; iptr; if (!s || RSTRING(str)->len == 0) return Qnil; send = s + RSTRING(str)->len; @@ -2344,6 +2340,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; @@ -2358,8 +2355,6 @@ rb_str_squeeze_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; -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- Michal Rokos Czech Technical University, Prague e-mail: m.rokos@sh.cvut.cz icq: 36118339 jabber: majkl@jabber.cz -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-