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 undefined behavior #130

Merged
merged 1 commit into from
Dec 16, 2020
Merged

Fix undefined behavior #130

merged 1 commit into from
Dec 16, 2020

Conversation

cfis
Copy link
Collaborator

@cfis cfis commented Dec 16, 2020

When the number of arguments to a method call is 0, then the asList vector has a size of zero. In that case, calling asList[0] is undefined behavior although with GCC it works fine in a debug mode (but per the C++ spec you aren't supposed to do that). On MSVC in debug mode that causes a crash. Instead, data() should be used instead.

Note a second way of solving this would be to specialize the call template like this:

template<>
inline Rice::Object Rice::Object::
call(Identifier id) const
{
return protect(rb_funcall, value(), id, 0);
}

I like that better but it seemed like a more invasive change.

* Fix undefined behavior. When the number of arguments to a method call is 0, then the asList vector has a size of zero. In that case, calling asList[0] is undefined behavior. On MSVC in debug mode that causes a crash. Instead, data() should be used instead. Note a second way of solving this would be to specialize the call template like this:

template<>
inline Rice::Object Rice::Object::
call(Identifier id) const
{
    return protect(rb_funcall, value(), id, 0);
}

I like that better but it seemed like a more invasive change.
@jasonroelofs
Copy link
Collaborator

Thanks for the fix! This change looks fine to me. CI passes fine (https://travis-ci.org/github/jasonroelofs/rice/builds/749963667) though that doesn't seem to be reporting here anymore, probably due to upcoming changes to Travis.

I will make some time this holiday to finalize a few things and actually get a new release out.

@jasonroelofs jasonroelofs merged commit e475be5 into ruby-rice:master Dec 16, 2020
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

Successfully merging this pull request may close these issues.

2 participants