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

mrb_string_value_cstr crashes if not given a string #2847

Closed
jbreeden opened this issue Jun 22, 2015 · 2 comments
Closed

mrb_string_value_cstr crashes if not given a string #2847

jbreeden opened this issue Jun 22, 2015 · 2 comments

Comments

@jbreeden
Copy link
Contributor

mrb_string_value_cstr does horrible things if given an argument that isn't actually a string (well, it tends to crash, anyway). That's all fine and well, since in C you should know what you're doing with your pointers. However, mrb_string_value_ptr avoids this by calling mrb_str_to_str on its argument. I'm just wondering if there's a reason that they behave differently. (Until now, I thought the _cstr version just stopped at the first null character, and _ptr was more suited to arbitrary char buffers. It wasn't clear to me that one is more type sensitive.)

I didn't see any issues/discussions on this yet. Maybe I'm the only one that's confused... O_o

@cremno
Copy link
Contributor

cremno commented Jun 22, 2015

You should create a PR. I agree that mrb_str_to_str() has to be called. I think most mruby users call mrb_str_to_cstr() instead which is very strict (just MRB_TT_STRING) and (or rather but?) creates a copy. Also this function is only used once in mruby and there it's preceded by a check. That's probably why nobody has found this bug before.

However there's something else: mrb_str_to_str() does much more than rb_str_to_str(). Is this intended, @matz?

mruby/src/string.c

Lines 628 to 641 in e344c6a

MRB_API mrb_value
mrb_str_to_str(mrb_state *mrb, mrb_value str)
{
mrb_value s;
if (!mrb_string_p(str)) {
s = mrb_check_convert_type(mrb, str, MRB_TT_STRING, "String", "to_str");
if (mrb_nil_p(s)) {
s = mrb_convert_type(mrb, str, MRB_TT_STRING, "String", "to_s");
}
return s;
}
return str;
}

https://github.com/ruby/ruby/blob/a02a3f46496210ca401fd74585993c8754cbe91c/string.c#L1087-L1091

@jbreeden
Copy link
Contributor Author

Sounds good. I'm heading to work, I'll make the PR when I get back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants