Re: [Cleanup, fixes] string.c
From:
Michal Rokos <m.rokos@...>
Date:
2002-09-11 16:28:25 UTC
List:
ruby-core #482
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; i<argc; i++) {
VALUE s = argv[i];
@@ -2302,8 +2300,6 @@ rb_str_delete_bang(argc, argv, str)
tr_setup_table(s, squeez, init);
init = 0;
}
-
- rb_str_modify(str);
s = t = RSTRING(str)->ptr;
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
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-