Skip to content

Conversation

@stoivo
Copy link

@stoivo stoivo commented Apr 25, 2023

  • Undefine allocation function for C extension class

Since Ruby 3.2 a new warning is printed when a Ruby class created in a C extension does not specify an allocate function or undefine it.

warning: undefining the allocator of T_DATA class FileMagic (WarningHandlers::Ruby::Warning)

From my understanding, we only need to define an allocate function if the class uses a C struct and stores any values on it. Our classes don't do that in C, that's done in our Rust extension.

Closes #909

Resources

https://bugs.ruby-lang.org/issues/18007
https://github.com/ruby/ruby/blob/6963f8f743b42f9004a0879cd66c550f18987352/doc/extension.rdoc#label-Write+the+C+Code https://ruby-doc.org/core-3.1.1/doc/extension_rdoc.html#label-C+struct+to+Ruby+object

rails-sqlserver/tiny_tds#515 https://groups.google.com/g/sequel-talk/c/K0J80s4wGJU/m/BT-6FFhrAgAJ

Other MR doing similar change

vmg/redcarpet#721
appsignal/appsignal-ruby#917

@kevingriffin
Copy link

Hi! I had a look at this for our usecase. It seems valid for the FileMagic class, as it shouldn't be allocated, but FileMagicError does get used like a regular class. I see these two cases:

  if ((ms = magic_open(NUM2INT(args[0]))) == NULL) {
    rb_raise(rb_FileMagicError,
      "failed to initialize magic cookie (%d)", errno || -1);
  }

  if (magic_load(ms, NULL) == -1) {
    rb_raise(rb_FileMagicError,
      "failed to load database: %s", magic_error(ms));
  }

in which case, I'm not sure this change is valid. Have you tested these error cases?

@stoivo
Copy link
Author

stoivo commented Jun 27, 2023

No, I have haven't tested it. I don't know how to either. My knowledge of C is low so Im not sure what I can do to test it

@bforma
Copy link

bforma commented Oct 16, 2023

Any update on this? I've tried this and it works for us. The warning is no longer shown.

@namely-sachin
Copy link

I am facing similar issue.

@namely-sachin
Copy link

Please merge this PR and release a new version of gem.

* Undefine allocation function for C extension class

Since Ruby 3.2 a new warning is printed when a Ruby class created in a C
extension does not specify an allocate function or undefine it.

```
/gem/lib/appsignal/transaction.rb:98: warning: undefining the allocator of T_DATA class Appsignal::Extension::Transaction
/gem/lib/appsignal/utils/data.rb:19: warning: undefining the allocator of T_DATA class Appsignal::Extension::Data
```

From my understanding, we only need to define an allocate function if
the class uses a C struct and stores any values on it. Our classes don't
do that in C, that's done in our Rust extension.

Closes #909

## Resources

https://bugs.ruby-lang.org/issues/18007
https://github.com/ruby/ruby/blob/6963f8f743b42f9004a0879cd66c550f18987352/doc/extension.rdoc#label-Write+the+C+Code
https://ruby-doc.org/core-3.1.1/doc/extension_rdoc.html#label-C+struct+to+Ruby+object

rails-sqlserver/tiny_tds#515
https://groups.google.com/g/sequel-talk/c/K0J80s4wGJU/m/BT-6FFhrAgAJ

## Other MR doing similar change
vmg/redcarpet#721
appsignal/appsignal-ruby#917
@stoivo
Copy link
Author

stoivo commented Jan 29, 2024

@kevingriffin, We did some testing on our side and we think you are right. I should remove some of my changes

@Lykos
Copy link

Lykos commented Feb 5, 2024

I just created issue #47 about this problem and now I saw this pull request fixes it. Not that I have anything to say, but I am supportive of fixing this!

@araccaine
Copy link

Sorry for bumping this again, but are there chances that this will be fixed?

@olegantonyan
Copy link

I ended up wrapping FileMagic calls in silence_warnings

@blackwinter
Copy link
Owner

It appears to be fixed in @flori's fork, so maybe he's going to release a new version.

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.

8 participants