Add a new mrb_fiber_new() with MRB_API#6097
Conversation
| mrb_fiber_new(mrb_state *mrb, const struct RProc *p) | ||
| { | ||
| struct RClass *c = mrb_class_get_id(mrb, MRB_SYM(Fiber)); | ||
| if (MRB_INSTANCE_TT(c) != MRB_TT_FIBER) { |
There was a problem hiding this comment.
When MRB_INSTANCE_TT(c) != MRB_TT_FIBER, MRB_OBJ_ALLOC will cause TypeError.
Do we really need this check?
There was a problem hiding this comment.
Yes, I consider it necessary.
% cat 1.c
#include <mruby.h>
int
main(int argc, char *argv[])
{
mrb_state *mrb = mrb_open();
struct RFiber *o = MRB_OBJ_ALLOC(mrb, MRB_TT_FIBER, mrb->object_class);
mrb_p(mrb, mrb_obj_value(o));
mrb_close(mrb);
return 0;
}
% $(bin/mruby-config --cc --cflags --ldflags --libs) 1.c && ./a.out
#<Object:0x823fcf720>This is because the instance type tag of the Object class is MRB_TT_FALSE and the ttype parameter of mrb_obj_alloc() is rarely checked in this case.
Lines 477 to 490 in 9e50d67
Or is it better to take the same approach as Random?
9b9424f#diff-bf9e7692602cba54ba62bd71b76c439be9c9b71c6bb8d51ab996fc91cf1ee807
There was a problem hiding this comment.
I am going to review the condition in mrb_obj_alloc().
Besides that, I don't think we should take Random approach. Random uses TT_ISTRUCT type that may be used by other classes. In contrast, TT_FIBER is only used by Fiber class.
There was a problem hiding this comment.
I don't know if this will help, but I have an experimental branch that I am suspending in the middle of a work in progress to make mrb_obj_alloc() more rigorous.
master...dearblue:mruby:wip/alloc-strict
The reason I am suspending it is because of compatibility concerns and at least my mruby-lz4 needs to be modified.
There was a problem hiding this comment.
Ok, I will merge this PR as it is. Then I will try to update mrb_obj_allocate.
2 participants
No description provided.