Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix String::append, UnMarshaller::get_string, and String::create.

Kill off the wacky char* operator overload and replace it with ->byte_address()
  • Loading branch information...
commit 7e36fb1300b4ae2306ad03984eb30c4604fea2fd 1 parent bcf8924
@wilson wilson authored
View
8 vm/builtin/nativefunction.cpp
@@ -119,7 +119,7 @@ namespace rubinius {
/* path is a string like 'ext/gzip', we turn that into 'ext/gzip.so'
or whatever the library suffix is. */
memset(sys_name, 0, 128);
- strlcpy(sys_name, *path, sizeof(sys_name));
+ strlcpy(sys_name, path->byte_address(), sizeof(sys_name));
strlcat(sys_name, LIBSUFFIX, sizeof(sys_name));
/* Open it up. If this fails, then we just pretend like
@@ -128,7 +128,7 @@ namespace rubinius {
if(!lib) {
#ifdef LIBSUFFIX2
memset(sys_name, 0, 128);
- strlcpy(sys_name, *path, sizeof(sys_name));
+ strlcpy(sys_name, path->byte_address(), sizeof(sys_name));
strlcat(sys_name, LIBSUFFIX2, sizeof(sys_name));
lib = xdlopen(sys_name);
@@ -139,9 +139,9 @@ namespace rubinius {
}
}
- ep = xdlsym(lib, *name);
+ ep = xdlsym(lib, name->byte_address());
if(!ep) {
- ep = xdlsym2(*name);
+ ep = xdlsym2(name->byte_address());
}
return ep;
}
View
4 vm/builtin/object.cpp
@@ -459,12 +459,12 @@ namespace rubinius {
void inspect(STATE, OBJECT obj) {
String* name = obj->class_object(state)->name->to_str(state);
- std::cout << "#<" << (char*)*name << ":" << (void*)obj << ">\n";
+ std::cout << "#<" << name->byte_address() << ":" << (void*)obj << ">\n";
}
void inspect(STATE, SYMBOL sym) {
String* name = sym->to_str(state);
- std::cout << ":" << (char*)*name << "\n";
+ std::cout << ":" << name->byte_address() << "\n";
}
void Object::cleanup(STATE) {
View
6 vm/builtin/regexp.cpp
@@ -118,7 +118,7 @@ namespace rubinius {
int err, num_names, kcode;
pat = (UChar*)pattern->byte_address(state);
- end = pat + pattern->size(state);
+ end = pat + pattern->size();
/* Ug. What I hate about the onig API is that there is no way
to define how to allocate the reg, onig_new does it for you.
@@ -214,7 +214,7 @@ namespace rubinius {
region = onig_region_new();
- max = string->size(state);
+ max = string->size();
str = (UChar*)string->byte_address(state);
if(!RTEST(forward)) {
@@ -241,7 +241,7 @@ namespace rubinius {
region = onig_region_new();
- max = string->size(state);
+ max = string->size();
str = (UChar*)string->byte_address(state);
beg = onig_match(REG(data), str, str + max, str + start->to_native(), region,
View
46 vm/builtin/string.cpp
@@ -15,30 +15,22 @@
namespace rubinius {
- size_t String::size(STATE) {
- return num_bytes->to_native();
- }
-
+ // String::size returns the actual number of bytes, without consideration
+ // for a trailing null byte. String::create(state, "foo")->size() is 3.
size_t String::size() {
return num_bytes->to_native();
}
- /* TODO: since we're technically say it's ok to change this, we might
- * want to copy it first. */
- String::operator char *() {
- return (char*)(data->bytes);
- }
-
String* String::allocate(STATE, FIXNUM size) {
String *so;
- size_t bytes = size->to_native();
so = (String*)state->om->new_object(G(string), String::fields);
- SET(so, num_bytes, Fixnum::from(bytes));
+ SET(so, num_bytes, size);
SET(so, characters, so->num_bytes);
SET(so, encoding, Qnil);
+ size_t bytes = size->to_native();
OBJECT ba = ByteArray::create(state, bytes);
ba->bytes[bytes] = 0;
@@ -47,10 +39,13 @@ namespace rubinius {
return so;
}
+ /* +bytes+ should NOT attempt to take the trailing null into account
+ * +bytes+ is the number of 'real' characters in the string
+ */
String* String::create(STATE, const char* str, size_t bytes) {
String *so;
- if(!bytes) bytes = strlen(str);
+ if(bytes == 0) bytes = strlen(str);
so = (String*)state->om->new_object(G(string), String::fields);
@@ -76,7 +71,7 @@ namespace rubinius {
return (hashval)as<Integer>(hash)->to_native();
}
bp = (unsigned char*)(data->bytes);
- size_t sz = size(state);
+ size_t sz = size();
hashval h = hash_str(bp, sz);
SET(this, hash, Integer::from(state, h));
@@ -148,21 +143,7 @@ namespace rubinius {
}
String* String::append(STATE, String* other) {
- if(shared) unshare(state);
-
- size_t new_size = size() + other->size();
-
- ByteArray *d2 = ByteArray::create(state, new_size + 1);
- std::memcpy(d2->bytes, data->bytes, size());
- std::memcpy(d2->bytes + size(), other->data->bytes, other->size());
-
- d2->bytes[new_size] = 0;
-
- num_bytes = Integer::from(state, new_size);
- SET(this, data, d2);
- SET(this, hash, Qnil);
-
- return this;
+ return append(state, other->byte_address());
}
String* String::append(STATE, const char* other) {
@@ -171,13 +152,16 @@ namespace rubinius {
size_t len = strlen(other);
size_t new_size = size() + len;
+ // Leave one extra byte of room for the trailing null
ByteArray *d2 = ByteArray::create(state, new_size + 1);
std::memcpy(d2->bytes, data->bytes, size());
+ // Append on top of the null byte at the end of s1, not after it
std::memcpy(d2->bytes + size(), other, len);
+ // This looks like it is off by one, but think about it.
d2->bytes[new_size] = 0;
- num_bytes = Integer::from(state, new_size);
+ SET(this, num_bytes, Integer::from(state, new_size));
SET(this, data, d2);
SET(this, hash, Qnil);
@@ -436,7 +420,7 @@ namespace rubinius {
void String::Info::show(STATE, OBJECT self) {
String* str = as<String>(self);
- std::cout << *str << std::endl;
+ std::cout << str->byte_address() << std::endl;
}
}
View
2  vm/builtin/string.hpp
@@ -86,8 +86,6 @@ namespace rubinius {
// Ruby.primitive :string_pattern
static String* pattern(STATE, OBJECT self, FIXNUM size, OBJECT pattern);
- operator char *();
-
class Info : public TypeInfo {
public:
BASIC_TYPEINFO(TypeInfo)
View
9 vm/builtin/task.cpp
@@ -170,8 +170,8 @@ namespace rubinius {
probe->lookup_failed(this, msg);
}
std::stringstream ss;
- ss << "unable to locate any method '" << *msg.send_site->name->to_str(state) <<
- "' from '" << *msg.lookup_from->name->to_str(state) << "'";
+ ss << "unable to locate any method '" << msg.send_site->name->to_str(state)->byte_address() <<
+ "' from '" << msg.lookup_from->name->to_str(state)->byte_address() << "'";
Assertion::raise((char*)ss.str().c_str());
}
@@ -429,7 +429,10 @@ namespace rubinius {
static Class* check_superclass(STATE, Class* cls, OBJECT super) {
if(super->nil_p()) return cls;
if(cls->superclass != super) {
- std::cout << "mismatch: " << *cls->name->to_str(state) << " != " << *as<Class>(super)->name->to_str(state) << "\n";
+ std::cout << "mismatch: "
+ << cls->name->to_str(state)->byte_address()
+ << " != " << as<Class>(super)->name->to_str(state)->byte_address()
+ << "\n";
TypeError::raise(Class::type, super, "superclass mismatch");
}
View
2  vm/codegen/field_extract.rb
@@ -596,7 +596,7 @@ def parse_stream(f)
f.puts <<-EOF
std::string msg = std::string(\"Unable to resolve primitive: \") +
- (char*)*name->to_str(state);
+ name->to_str(state)->byte_address();
std::cout << msg << std::endl;
return &Primitives::unknown_primitive;
// commented out while we have soft primitive failures
View
8 vm/environment.cpp
@@ -96,16 +96,16 @@ namespace rubinius {
void Environment::set_rubinius_constants() {
Module* rubinius = GO(rubinius).get();
- String* ruby_version = String::create(state, "1.8.6", 6);
+ String* ruby_version = String::create(state, "1.8.6");
rubinius->set_const(state, "RUBY_VERSION", ruby_version);
- String* ruby_patchlevel = String::create(state, "111", 4);
+ String* ruby_patchlevel = String::create(state, "111");
rubinius->set_const(state, "RUBY_PATCHLEVEL", ruby_patchlevel);
- String* ruby_engine = String::create(state, "rbx", 4);
+ String* ruby_engine = String::create(state, "rbx");
rubinius->set_const(state, "RUBY_ENGINE", ruby_engine);
- String* rbx_version = String::create(state, "0.9.0", 6);
+ String* rbx_version = String::create(state, "0.9.0");
rubinius->set_const(state, "RBX_VERSION", rbx_version);
}
}
View
20 vm/instructions.rb
@@ -3200,12 +3200,22 @@ def passed_blockarg(count)
def string_append
<<-CODE
- OBJECT t1 = stack_pop();
- OBJECT t2 = stack_pop();
- String* s1 = as<String>(t1);
- String* s2 = as<String>(t2);
+ String* s1 = as<String>(stack_pop());
+ String* s2 = as<String>(stack_pop());
s1->append(state, s2);
- stack_push(t1);
+ stack_push(s1);
+ CODE
+ end
+
+ def test_string_append
+ <<-CODE
+ String* s1 = String::create(state, "first");
+ String* s2 = String::create(state, " second");
+ task->push(s2);
+ task->push(s1);
+ run();
+ TS_ASSERT_EQUALS(task->stack_at(0)->class_object(state), G(string));
+ TS_ASSERT_SAME_DATA(as<String>(task->pop())->byte_address(), "first second", 13);
CODE
end
View
2  vm/llvm.cpp
@@ -280,7 +280,7 @@ namespace rubinius {
}
void VMLLVMMethod::compile(STATE) {
- char* name = *original->name->to_str(state);
+ char* name = original->name->to_str(state)->byte_address();
Function* func = create_function(name);
Function::arg_iterator args = func->arg_begin();
View
11 vm/marshal.cpp
@@ -39,17 +39,18 @@ namespace rubinius {
void Marshaller::set_string(String* str) {
stream << "s" << endl << str->size() << endl;
- stream.write((char*)*str, str->size()) << endl;
+ stream.write(str->byte_address(), str->size()) << endl;
}
String* UnMarshaller::get_string() {
size_t count;
stream >> count;
- String* str = String::create(state, NULL, count + 1);
+ // String::create adds room for a trailing null on its own
+ String* str = String::create(state, NULL, count);
stream.get(); // read off newline
- stream.read((char*)*str, count);
+ stream.read(str->byte_address(), count);
stream.get(); // read off newline
return str;
@@ -58,7 +59,7 @@ namespace rubinius {
void Marshaller::set_symbol(SYMBOL sym) {
String* str = sym->to_str(state);
stream << "x" << endl << str->size() << endl;
- stream.write((char*)*str, str->size()) << endl;
+ stream.write(str->byte_address(), str->size()) << endl;
}
SYMBOL UnMarshaller::get_symbol() {
@@ -76,7 +77,7 @@ namespace rubinius {
void Marshaller::set_sendsite(SendSite* ss) {
String* str = ss->name->to_str(state);
stream << "S" << endl << str->size() << endl;
- stream.write((char*)*str, str->size()) << endl;
+ stream.write(str->byte_address(), str->size()) << endl;
}
SendSite* UnMarshaller::get_sendsite() {
View
2  vm/primitives.cpp
@@ -6,7 +6,7 @@ namespace rubinius {
bool Primitives::unknown_primitive(STATE, VMExecutable* exec, Task* task, Message& msg) {
std::cout << "\n";
state->print_backtrace();
- std::cout << "Called unbound or invalid primitive from: " << *msg.name->to_str(state) <<"\n";
+ std::cout << "Called unbound or invalid primitive from: " << msg.name->to_str(state)->byte_address() <<"\n";
abort();
return false;
}
View
6 vm/probes.cpp
@@ -14,19 +14,19 @@ namespace rubinius {
void TaskProbe::start_method(Task* task, Message& msg) {
if(!verbose) return;
- std::cout << "[Sending: '" << *msg.send_site->name->to_str(task->state) <<
+ std::cout << "[Sending: '" << msg.send_site->name->to_str(task->state)->byte_address() <<
"']" << std::endl;
}
void TaskProbe::lookup_failed(Task* task, Message& msg) {
if(!verbose) return;
- std::cout << "[Unable to find: '" << *msg.send_site->name->to_str(task->state) <<
+ std::cout << "[Unable to find: '" << msg.send_site->name->to_str(task->state)->byte_address() <<
"']" << std::endl;
}
void TaskProbe::added_method(Task* task, Module* mod, SYMBOL name, CompiledMethod *meth) {
if(!verbose) return;
- std::cout << "[Added method '" << *name->to_str(task->state) << "']" << std::endl;
+ std::cout << "[Added method '" << name->to_str(task->state)->byte_address() << "']" << std::endl;
}
}
View
2  vm/symboltable.cpp
@@ -41,7 +41,7 @@ namespace rubinius {
String* SymbolTable::lookup_string(STATE, Symbol* sym) {
std::string str = strings[sym->index()];
- return String::create(state, str.c_str(), str.size());
+ return String::create(state, str.c_str());
}
size_t SymbolTable::size() {
View
2  vm/test/test_bignum.hpp
@@ -604,7 +604,7 @@ class TestBignum : public CxxTest::TestSuite {
Bignum* b = as<Bignum>(Bignum::from_string(state, buf, 10));
String* s = b->to_s(state, Fixnum::from(10));
- TS_ASSERT_EQUALS(std::string(buf), (char*)*s);
+ TS_ASSERT_EQUALS(std::string(buf), s->byte_address());
}
void test_size() {
View
2  vm/test/test_fixnum.hpp
@@ -474,7 +474,7 @@ class TestFixnum : public CxxTest::TestSuite {
void test_to_s() {
String* n = Fixnum::from(86545)->to_s(state);
- TS_ASSERT_EQUALS(std::string("86545"), (char*)*n);
+ TS_ASSERT_EQUALS(std::string("86545"), n->byte_address());
}
void test_uncastable_object_throws_exception() {
View
4 vm/test/test_memorypointer.hpp
@@ -293,7 +293,7 @@ class TestMemoryPointer : public CxxTest::TestSuite {
String* so = as<String>(obj);
- TS_ASSERT(!strncmp(str, *so, 4));
+ TS_ASSERT(!strncmp(str, so->byte_address(), 4));
}
void test_get_field_string_thats_null() {
@@ -318,7 +318,7 @@ class TestMemoryPointer : public CxxTest::TestSuite {
TS_ASSERT(ary->get(state, 0)->check_type(StringType));
String *so = as<String>(ary->get(state, 0));
- TS_ASSERT(!strncmp(str, *so, 4));
+ TS_ASSERT(!strncmp(str, so->byte_address(), 4));
TS_ASSERT(ary->get(state, 1)->check_type(MemPtrType));
MemoryPointer* mp = as<MemoryPointer>(ary->get(state, 1));
View
4 vm/test/test_nativefunction.hpp
@@ -559,7 +559,7 @@ class TestNativeFunction : public CxxTest::TestSuite {
OBJECT out = func->call(state, msg);
TS_ASSERT(kind_of<String>(out));
- TS_ASSERT_EQUALS((char*)*(as<String>(out)), std::string("whatever"));
+ TS_ASSERT_EQUALS(as<String>(out)->byte_address(), std::string("whatever"));
input = Array::create(state, 1);
input->set(state, 0, Qnil);
@@ -598,7 +598,7 @@ class TestNativeFunction : public CxxTest::TestSuite {
TS_ASSERT(kind_of<String>(o1));
TS_ASSERT(kind_of<MemoryPointer>(o2));
- TS_ASSERT_EQUALS((char*)*(as<String>(o1)), std::string("whatever"));
+ TS_ASSERT_EQUALS(as<String>(o1)->byte_address(), std::string("whatever"));
TS_ASSERT(strcmp((char*)(as<MemoryPointer>(o2)->pointer), "whatever") == 0);
input = Array::create(state, 1);
View
20 vm/test/test_string.hpp
@@ -22,12 +22,13 @@ class TestString : public CxxTest::TestSuite {
void test_create() {
TS_ASSERT(state->globals.string->has_ivars->false_p());
str = String::create(state, "blah");
- TS_ASSERT_EQUALS(str->size(state), 4U);
+ TS_ASSERT_EQUALS(str->size(), 4U);
}
void test_create2() {
str = String::create(state, "blah", 2);
- TS_ASSERT_EQUALS(str->size(state), 2U);
+ TS_ASSERT_EQUALS(str->size(), 2U);
+ TS_ASSERT_SAME_DATA("bl", str->byte_address(), 3);
}
void test_hash_string() {
@@ -64,33 +65,34 @@ class TestString : public CxxTest::TestSuite {
str->unshare(state);
TS_ASSERT(str->data != str2->data);
- TS_ASSERT_EQUALS(std::string("blah"), (char*)*str);
+ TS_ASSERT_EQUALS(std::string("blah"), str->byte_address());
}
void test_append() {
- str = String::create(state, "blah");
+ str = String::create(state, "first");
str->hash_string(state);
TS_ASSERT(str->hash != Qnil);
- str->append(state, String::create(state, " foo"));
+ str->append(state, String::create(state, " second"));
TS_ASSERT_EQUALS(str->hash, Qnil);
- TS_ASSERT_EQUALS(std::string("blah foo"), (char*)*str);
+ TS_ASSERT_EQUALS(str->size(), 12U);
+ TS_ASSERT_SAME_DATA("first second", str->byte_address(), 13);
}
void test_append_with_charstar() {
str = String::create(state, "blah");
str->append(state, " foo");
- TS_ASSERT_EQUALS(std::string("blah foo"), (char*)*str);
+ TS_ASSERT_SAME_DATA("blah foo", str->byte_address(), 9);
}
void test_add() {
str = String::create(state, "blah");
String* str2 = str->add(state, String::create(state, " foo"));
- TS_ASSERT_EQUALS(std::string("blah foo"), (char*)*str2);
- TS_ASSERT_EQUALS(std::string("blah"), (char*)*str);
+ TS_ASSERT_EQUALS(std::string("blah foo"), str2->byte_address());
+ TS_ASSERT_EQUALS(std::string("blah"), str->byte_address());
}
View
4 vm/test/test_unmarshal.hpp
@@ -82,7 +82,7 @@ class TestUnMarshal : public CxxTest::TestSuite {
TS_ASSERT(kind_of<String>(obj));
String *str = as<String>(obj);
- TS_ASSERT_EQUALS(std::string(*str), "blah");
+ TS_ASSERT_EQUALS(std::string(str->byte_address()), "blah");
}
void test_symbol() {
@@ -114,7 +114,7 @@ class TestUnMarshal : public CxxTest::TestSuite {
TS_ASSERT_EQUALS(ary->get(state, 1), state->symbol("foo"));
String* str = as<String>(ary->get(state, 2));
- TS_ASSERT_EQUALS(std::string("blah"), (char*)*str);
+ TS_ASSERT_EQUALS(std::string("blah"), str->byte_address());
}
void test_tuple() {
View
6 vm/type_info.cpp
@@ -50,13 +50,13 @@ namespace rubinius {
if(FIXNUM i = try_as<Fixnum>(self)) {
std::cout << i->to_native() << std::endl;
} else if(Bignum* b = try_as<Bignum>(self)) {
- std::cout << *b->to_s(state, Fixnum::from(10)) << std::endl;
+ std::cout << b->to_s(state, Fixnum::from(10))->byte_address() << std::endl;
} else if(Float* f = try_as<Float>(self)) {
std::cout << f->val << std::endl;
} else if(String* str = try_as<String>(self)) {
- std::cout << *str << std::endl;
+ std::cout << str->byte_address() << std::endl;
} else if(SYMBOL sym = try_as<Symbol>(self)) {
- std::cout << ":" << *sym->to_str(state) << std::endl;
+ std::cout << ":" << sym->to_str(state)->byte_address() << std::endl;
} else {
inspect(state, self);
}
View
10 vm/vm.cpp
@@ -150,7 +150,7 @@ namespace rubinius {
void VM::inspect(OBJECT obj) {
if(obj->symbol_p()) {
String* str = as<Symbol>(obj)->to_str(this);
- std::cout << "<Symbol :" << (char*)*str << ">" << std::endl;
+ std::cout << "<Symbol :" << str->byte_address() << ">" << std::endl;
} else if(obj->fixnum_p()) {
std::cout << "<Fixnum " << as<Fixnum>(obj)->to_native() << ">" << std::endl;
} else {
@@ -175,17 +175,17 @@ namespace rubinius {
std::cout << "__block__";
} else {
// HACK reports Object#[] instead of Hash::[], etc
- std::cout << *ctx->module->name->to_str(this) << "#";
+ std::cout << ctx->module->name->to_str(this)->byte_address() << "#";
SYMBOL name = try_as<Symbol>(ctx->name);
if(name) {
- std::cout << *name->to_str(this);
+ std::cout << name->to_str(this)->byte_address();
} else {
- std::cout << *ctx->cm->name->to_str(this);
+ std::cout << ctx->cm->name->to_str(this)->byte_address();
}
}
- std::cout << ":" << ctx->line() << " in " << *ctx->cm->file->to_str(this);
+ std::cout << ":" << ctx->line() << " in " << ctx->cm->file->to_str(this)->byte_address();
std::cout << "\n";
ctx = ctx->sender;
Please sign in to comment.
Something went wrong with that request. Please try again.