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

Already on GitHub? Sign in to your account

Use rb_ary_entry instead of RARRAY_PTR #877

Merged
merged 1 commit into from May 15, 2013

Conversation

Projects
None yet
2 participants
Contributor

dbussink commented Mar 26, 2013

For MRI, this doesn't make a difference, but on Rubinius this prevents
having to setup a backing store for copying over the internal array
pointers. This was the only case where RARRAY_PTR was used, other places
already use rb_ary_entry, so it should be a good change for consistency
as well.

@dbussink dbussink Use a rb_ary_entry and not RARRAY_PTR()
For MRI, this doesn't make a difference, but on Rubinius this prevents
having to setup a backing store for copying over the internal array
pointers. This was the only case where RARRAY_PTR was used, other places
already use rb_ary_entry, so it should be a good change for consistency
as well.
2affe7c
Contributor

dbussink commented May 9, 2013

Any way to get this merged? Or is there anything that would hold this back?

Owner

leejarvis commented May 15, 2013

@dbussink Getting a segfault on rbx-18mode. I'm happy to merge this once this runs green but @flavorjones might want to wait until after 1.6.0 is released

Contributor

dbussink commented May 15, 2013

I don't see how this could be related to the Rubinius segfault, since I actually changed this because it works better for Rubinius (and ran the tests locally). I guess they were due to some other issue that existed when I sent this pull request that has been fixed in the mean while.

Also this change is really minor, so don't really see how it would affect a release, but it's not my decision of course :).

Owner

leejarvis commented May 15, 2013

@dbussink I agree on both counts. I'll run this against travis again so it's running green, I'm happy to merge before the release :) will catch up with this later. Thanks

@leejarvis leejarvis added a commit that referenced this pull request May 15, 2013

@leejarvis leejarvis Merge pull request #877 from rubinius/master
Use rb_ary_entry instead of RARRAY_PTR
ff9eeea

@leejarvis leejarvis merged commit ff9eeea into sparklemotion:master May 15, 2013

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment