ruby-filemagic
ruby-filemagic copied to clipboard
blackwinter
Undefine allocation function for C extension class
- 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
https://github.com/rails-sqlserver/tiny_tds/issues/515 https://groups.google.com/g/sequel-talk/c/K0J80s4wGJU/m/BT-6FFhrAgAJ
Other MR doing similar change
https://github.com/vmg/redcarpet/pull/721 https://github.com/appsignal/appsignal-ruby/pull/917
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?
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
Any update on this? I've tried this and it works for us. The warning is no longer shown.
@kevingriffin, We did some testing on our side and we think you are right. I should remove some of my changes
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!
