arthurnn / memcached

A Ruby interface to the libmemcached C client
Academic Free License v3.0
432 stars 125 forks source link

Use rb_external_str_new_with_enc to reduce coupling to RString struct #199

Closed dylanahsmith closed 3 years ago

dylanahsmith commented 3 years ago

@casperisfine for review

Problem

When running the test suite using ruby 3, I was getting a couple of these warnings

RSTRING_PTR is returning NULL!! SIGSEGV is highly expected to follow immediately. If you could reproduce, attach your debugger here, and look at the passed string.

I found one instance coming from test/unit/memcached_test.rb test_get_multi_empty_string which sets and gets an empty raw string value from the cache. It looks like memcached is using a NULL string pointer when the length is 0, which makes sense. Unfortunately, the ruby string reference is setup by setting the RString struct values directly, leading to this problem, as well as potentially leading to more problems in the future.

Solution

Use rb_external_str_new_with_enc to create a ruby string from an external C string, which has a condition for the length being 0 and avoids this internal ruby coupling.

casperisfine commented 3 years ago

Nevermind, it default to rb_default_external_encoding().