mirrors/mkxp-z
1
0
Fork 0
mirror of https://github.com/mkxp-z/mkxp-z.git synced 2025-04-29 19:44:49 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

Properly handle uninitialized and reinitialized objects

Browse source 
In RGSS, uninitialized disposable objects (and Fonts, sort of) are technically disposed, and other objects (Tone, Color, Rect, and Table) are created with all values set to 0. It's also possible to reinitialize them, although reinitializing disposables leaks memory.

This commit causes MiniFFI and FileInt objects to improperly raise disposed errors if used while uninitialized, but that feels acceptable to me.
This commit is contained in:
Wayward Heart 2024-06-17 11:49:56 -05:00
parent 99ad4fa636
commit 130375b6d8
8 changed files with 164 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

57
binding/binding-util.h
View file

@ -198,20 +198,50 @@ template <rb_data_type_t *rbType> static VALUE classAllocate(VALUE klass) {
}
#endif
#if RAPI_FULL > 187
#define CLASS_ALLOCATE_PRE_INIT(Name, initializeFunc) \
static VALUE Name##AllocatePreInit(VALUE klass) { \
VALUE ret = classAllocate<& Name##Type>(klass); \
\
initializeFunc(0, 0, ret); \
\
return ret; \
}
#else
#define CLASS_ALLOCATE_PRE_INIT(Name, initializeFunc) \
static VALUE Name##AllocatePreInit(VALUE klass) { \
VALUE ret = Name##Allocate(klass); \
\
initializeFunc(0, 0, ret); \
\
return ret; \
}
#endif
template <class C> static void freeInstance(void *inst) {
delete static_cast<C *>(inst);
}
void raiseDisposedAccess(VALUE self);
template <class C> inline C *getPrivateData(VALUE self) {
template <class C> inline C *getPrivateDataNoRaise(VALUE self) {
#if RAPI_FULL > 187
C *c = static_cast<C *>(RTYPEDDATA_DATA(self));
return static_cast<C *>(RTYPEDDATA_DATA(self));
#else
C *c = static_cast<C *>(DATA_PTR(self));
return static_cast<C *>(DATA_PTR(self));
#endif
}
template <class C> inline C *getPrivateData(VALUE self) {
C *c = getPrivateDataNoRaise<C>(self);
if (!c) {
raiseRbExc(Exception(Exception::MKXPError, "No instance data for variable (missing call to super?)"));
//raiseRbExc(Exception(Exception::MKXPError, "No instance data for variable (missing call to super?)"));
/* FIXME: MiniFFI and FileInt don't have default allocations
* despite not being disposables. Should they be fixed,
* or just left with a misleading error message? */
raiseDisposedAccess(self);
}
return c;
}
@ -246,9 +276,28 @@ getPrivateDataCheck(VALUE self, const char *type)
}
static inline void setPrivateData(VALUE self, void *p) {
/* RGSS's behavior is to just leak memory if a disposable is reinitialized,
* with the original disposable being left permanently instantiated,
* but that's (1) bad, and (2) would currently cause memory access issues
* when things like a sprite's src_rect inevitably get GC'd, so we're not
* copying that. */
#if RAPI_FULL > 187
// Free the old value if it already exists (initialize called twice?)
if (RTYPEDDATA_DATA(self) && (RTYPEDDATA_DATA(self) != p)) {
/* RUBY_TYPED_NEVER_FREE == 0, and we don't use
* RUBY_TYPED_DEFAULT_FREE for our stuff, so just
* checking if it's truthy should be fine */
if (RTYPEDDATA_TYPE(self)->function.dfree)
(*RTYPEDDATA_TYPE(self)->function.dfree)(RTYPEDDATA_DATA(self));
}
RTYPEDDATA_DATA(self) = p;
#else
// Free the old value if it already exists (initialize called twice?)
if (DATA_PTR(self) && (DATA_PTR(self) != p)) {
/* As above, just check if it's truthy */
if (RDATA(self)->dfree)
(*RDATA(self)->dfree)(DATA_PTR(self));
}
DATA_PTR(self) = p;
#endif
}

55
binding/bitmap-binding.cpp
View file

@ -141,9 +141,11 @@ RB_METHOD_GUARD(bitmapBlt) {
&opacity RB_ARG_END);
src = getPrivateDataCheck<Bitmap>(srcObj, BitmapType);
srcRect = getPrivateDataCheck<Rect>(srcRectObj, RectType);
if (src) {
srcRect = getPrivateDataCheck<Rect>(srcRectObj, RectType);
GFX_GUARD_EXC(b->blt(x, y, *src, srcRect->toIntRect(), opacity););
GFX_GUARD_EXC(b->blt(x, y, *src, srcRect->toIntRect(), opacity););
}
return self;
}
@ -164,11 +166,13 @@ RB_METHOD_GUARD(bitmapStretchBlt) {
&opacity RB_ARG_END);
src = getPrivateDataCheck<Bitmap>(srcObj, BitmapType);
destRect = getPrivateDataCheck<Rect>(destRectObj, RectType);
srcRect = getPrivateDataCheck<Rect>(srcRectObj, RectType);
GFX_GUARD_EXC(b->stretchBlt(destRect->toIntRect(), *src, srcRect->toIntRect(),
opacity););
if (src) {
destRect = getPrivateDataCheck<Rect>(destRectObj, RectType);
srcRect = getPrivateDataCheck<Rect>(srcRectObj, RectType);
GFX_GUARD_EXC(b->stretchBlt(destRect->toIntRect(), *src, srcRect->toIntRect(),
opacity););
}
return self;
}
@ -332,7 +336,40 @@ RB_METHOD_GUARD(bitmapTextSize) {
}
RB_METHOD_GUARD_END
DEF_GFX_PROP_OBJ_VAL(Bitmap, Font, Font, "font")
RB_METHOD(BitmapGetFont) {
RB_UNUSED_PARAM;
checkDisposed<Bitmap>(self);
return rb_iv_get(self, "font");
}
RB_METHOD_GUARD(BitmapSetFont) {
rb_check_argc(argc, 1);
Bitmap *b = getPrivateData<Bitmap>(self);
VALUE propObj = *argv;
Font *prop = getPrivateDataCheck<Font>(propObj, FontType);
if (prop) {
GFX_GUARD_EXC(b->setFont(*prop);)
VALUE f = rb_iv_get(self, "font");
if (f) {
rb_iv_set(f, "name", rb_iv_get(propObj, "name"));
rb_iv_set(f, "size", rb_iv_get(propObj, "size"));
rb_iv_set(f, "bold", rb_iv_get(propObj, "bold"));
rb_iv_set(f, "italic", rb_iv_get(propObj, "italic"));
if (rgssVer >= 2) {
rb_iv_set(f, "shadow", rb_iv_get(propObj, "shadow"));
}
if (rgssVer >= 3) {
rb_iv_set(f, "outline", rb_iv_get(propObj, "outline"));
}
}
}
return propObj;
}
RB_METHOD_GUARD_END
RB_METHOD_GUARD(bitmapGradientFillRect) {
Bitmap *b = getPrivateData<Bitmap>(self);
@ -602,6 +639,8 @@ RB_METHOD_GUARD(bitmapAddFrame){
rb_scan_args(argc, argv, "11", &srcBitmap, &position);
Bitmap *src = getPrivateDataCheck<Bitmap>(srcBitmap, BitmapType);
if (!src)
raiseDisposedAccess(srcBitmap);
Bitmap *b = getPrivateData<Bitmap>(self);

6
binding/disposable-binding.h
View file

@ -30,8 +30,8 @@ template<class C>
RB_METHOD(disposableDispose)
{
RB_UNUSED_PARAM;
C *d = getPrivateData<C>(self);
C *d = getPrivateDataNoRaise<C>(self);
if (!d)
return Qnil;
@ -46,7 +46,7 @@ RB_METHOD(disposableIsDisposed)
{
RB_UNUSED_PARAM;
C *d = getPrivateData<C>(self);
C *d = getPrivateDataNoRaise<C>(self);
if (!d)
return Qtrue;

30
binding/etc-binding.cpp
View file

@ -107,7 +107,13 @@ EQUAL_FUN(Rect)
rb_get_args(argc, argv, param_t_s, &p1, &p2, &p3, &p4 RB_ARG_END); \
k = new Klass(p1, p2, p3, p4); \
} \
Klass *orig = getPrivateDataNoRaise<Klass>(self); \
if (orig) { \
*orig = *k; \
delete k; \
} else { \
setPrivateData(self, k); \
} \
return self; \
}
@ -208,11 +214,14 @@ INITCOPY_FUN(Tone)
INITCOPY_FUN(Color)
INITCOPY_FUN(Rect)
#if RAPI_FULL > 187
CLASS_ALLOCATE_PRE_INIT(Tone, ToneInitialize);
CLASS_ALLOCATE_PRE_INIT(Color, ColorInitialize);
CLASS_ALLOCATE_PRE_INIT(Rect, RectInitialize);
#define INIT_BIND(Klass) \
{ \
klass = rb_define_class(#Klass, rb_cObject); \
rb_define_alloc_func(klass, classAllocate<&Klass##Type>); \
rb_define_alloc_func(klass, Klass##AllocatePreInit); \
rb_define_class_method(klass, "_load", Klass##Load); \
serializableBindingInit<Klass>(klass); \
_rb_define_method(klass, "initialize", Klass##Initialize); \
@ -224,23 +233,6 @@ INITCOPY_FUN(Rect)
_rb_define_method(klass, "to_s", Klass##Stringify); \
_rb_define_method(klass, "inspect", Klass##Stringify); \
}
#else
#define INIT_BIND(Klass) \
{ \
klass = rb_define_class(#Klass, rb_cObject); \
rb_define_alloc_func(klass, Klass##Allocate); \
rb_define_class_method(klass, "_load", Klass##Load); \
serializableBindingInit<Klass>(klass); \
_rb_define_method(klass, "initialize", Klass##Initialize); \
_rb_define_method(klass, "initialize_copy", Klass##InitializeCopy); \
_rb_define_method(klass, "set", Klass##Set); \
_rb_define_method(klass, "==", Klass##Equal); \
_rb_define_method(klass, "===", Klass##Equal); \
_rb_define_method(klass, "eql?", Klass##Equal); \
_rb_define_method(klass, "to_s", Klass##Stringify); \
_rb_define_method(klass, "inspect", Klass##Stringify); \
}
#endif
#define MRB_ATTR_R(Class, attr) \
mrb_define_method(mrb, klass, #attr, Class##Get_##attr, MRB_ARGS_NONE())

10
binding/font-binding.cpp
View file

@ -88,7 +88,15 @@ RB_METHOD(fontInitialize) {
* However the same bug/behavior exists in all RM versions. */
rb_iv_set(self, "name", namesObj);
setPrivateData(self, f);
Font *orig = getPrivateDataNoRaise<Font>(self);
if (orig)
{
*orig = *f;
delete f;
f = orig;
} else {
setPrivateData(self, f);
}
/* Wrap property objects */
f->initDynAttribs();

26
binding/table-binding.cpp
View file

@ -57,9 +57,14 @@ RB_METHOD(tableInitialize) {
parseArgsTableSizes(argc, argv, &x, &y, &z);
Table *t = new Table(x, y, z);
Table *t = getPrivateDataNoRaise<Table>(self);
if (t) {
t->resize(x, y, z);
} else {
t = new Table(x, y, z);
setPrivateData(self, t);
setPrivateData(self, t);
}
return self;
}
@ -152,13 +157,20 @@ RB_METHOD_GUARD_END
MARSH_LOAD_FUN(Table)
INITCOPY_FUN(Table)
RB_METHOD(tableInitializeDefault) {
Table *t = new Table(0, 0, 0);
setPrivateData(self, t);
return self;
}
CLASS_ALLOCATE_PRE_INIT(Table, tableInitializeDefault);
void tableBindingInit() {
VALUE klass = rb_define_class("Table", rb_cObject);
#if RAPI_FULL > 187
rb_define_alloc_func(klass, classAllocate<&TableType>);
#else
rb_define_alloc_func(klass, TableAllocate);
#endif
rb_define_alloc_func(klass, TableAllocatePreInit);
serializableBindingInit<Table>(klass);

13
binding/tilemap-binding.cpp
View file

@ -35,7 +35,10 @@ DEF_TYPE_CUSTOMFREE(TilemapAutotiles, RUBY_TYPED_NEVER_FREE);
#endif
RB_METHOD(tilemapAutotilesSet) {
Tilemap::Autotiles *a = getPrivateData<Tilemap::Autotiles>(self);
Tilemap::Autotiles *a = getPrivateDataNoRaise<Tilemap::Autotiles>(self);
if (!a)
return self;
int i;
VALUE bitmapObj;
@ -93,12 +96,18 @@ RB_METHOD(tilemapInitialize) {
t->initDynAttribs();
/* Dispose the old autotiles if we're reinitializing.
* See the comment in setPrivateData for more info. */
VALUE autotilesObj = rb_iv_get(self, "autotiles");
if (autotilesObj != Qnil)
setPrivateData(autotilesObj, 0);
wrapProperty(self, &t->getAutotiles(), "autotiles", TilemapAutotilesType);
wrapProperty(self, &t->getColor(), "color", ColorType);
wrapProperty(self, &t->getTone(), "tone", ToneType);
VALUE autotilesObj = rb_iv_get(self, "autotiles");
autotilesObj = rb_iv_get(self, "autotiles");
VALUE ary = rb_ary_new2(7);
for (int i = 0; i < 7; ++i)

13
binding/tilemapvx-binding.cpp
View file

@ -58,10 +58,16 @@ RB_METHOD(tilemapVXInitialize) {
rb_iv_set(self, "viewport", viewportObj);
/* Dispose the old bitmap array if we're reinitializing.
* See the comment in setPrivateData for more info. */
VALUE autotilesObj = rb_iv_get(self, "bitmap_array");
if (autotilesObj != Qnil)
setPrivateData(autotilesObj, 0);
wrapProperty(self, &t->getBitmapArray(), "bitmap_array", BitmapArrayType,
rb_const_get(rb_cObject, rb_intern("Tilemap")));
VALUE autotilesObj = rb_iv_get(self, "bitmap_array");
autotilesObj = rb_iv_get(self, "bitmap_array");
VALUE ary = rb_ary_new2(9);
for (int i = 0; i < 9; ++i)
@ -106,7 +112,10 @@ DEF_GFX_PROP_I(TilemapVX, OX)
DEF_GFX_PROP_I(TilemapVX, OY)
RB_METHOD(tilemapVXBitmapsSet) {
TilemapVX::BitmapArray *a = getPrivateData<TilemapVX::BitmapArray>(self);
TilemapVX::BitmapArray *a = getPrivateDataNoRaise<TilemapVX::BitmapArray>(self);
if (!a)
return self;
int i;
VALUE bitmapObj;