Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix strlen for truffle objects to report the null terminated length if it is shorter than the full length. #98

Merged
merged 1 commit into from
Mar 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions spec/tags/optional/capi/string_tags.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
fails:C-API String function rb_str_buf_new returns a string whose bytes can be accessed by RSTRING_PTR
fails:C-API String function rb_str_buf_new returns a string that can be modified by rb_str_set_len
fails:C-API String function rb_str_new2 returns a new string object calling strlen on the passed C string
fails:C-API String function rb_str_new2 encodes the string with ASCII_8BIT
fails:C-API String function rb_str_new_cstr returns a new string object calling strlen on the passed C string
fails:C-API String function rb_str_new_cstr encodes the string with ASCII_8BIT
fails:C-API String function rb_str_encode accepts an encoding options Hash specifying replacement String
fails:C-API String function rb_str_subseq returns a byte-indexed substring
Expand Down
4 changes: 3 additions & 1 deletion truffleruby/src/main/c/cext/ruby.c
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,9 @@ VALUE rb_str_new(const char *string, long length) {

VALUE rb_str_new_cstr(const char *string) {
if (truffle_is_truffle_object((VALUE) string)) {
return (VALUE) truffle_invoke(RUBY_CEXT, "to_ruby_string", string);
VALUE ruby_string = (VALUE) truffle_invoke(RUBY_CEXT, "to_ruby_string", string);
int len = strlen(string);
Copy link
Member

@eregon eregon Mar 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be strlen(ruby_string)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. The only case I have changed for strlen is thatStringCharPointerAdpater#GET_SIZE will return the null terminated length so that it matches the semantics of char * strings, and it is that length we are interested in. I don't think it's a particularly good idea to call C functions which expected a char * with ruby string anyway, you're unlikely to get the answer you wanted, would certainly see inconsistencies between implementations, and should really get a warning (though I notice I didn't get one from Clang when I just tried it).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I was confused.

return (VALUE) truffle_invoke(ruby_string, "[]", 0, len);
} else {
return (VALUE) truffle_invoke(RUBY_CEXT, "rb_str_new_cstr", truffle_read_string(string));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,14 @@ protected Object access(StringCharPointerAdapter object) {
public static abstract class CharPointerGetSizeNode extends Node {

protected Object access(StringCharPointerAdapter stringCharPointerAdapter) {
return rope(stringCharPointerAdapter.getString()).byteLength();
byte[] bytes = rope(stringCharPointerAdapter.getString()).getBytes();
int i = 0;
for (;i < bytes.length; i++) {
if (bytes[i] == 0) {
break;
}
}
return i;
}

}
Expand Down