Make 'rb_get_args()' va_arg passing safer by introducing a termination marker

What can I say. I made a pact with the devil, and paid dearly.
Almost a whole day's worth of debugging, actually. Not again.

If this turns out to be slow we can always optimize the critical
parts (with no variable param count) later, or completely remove it.
This commit is contained in:
Jonas Kulla 2013-09-27 04:49:48 +02:00
parent 92525cd077
commit 4ff563725b
15 changed files with 92 additions and 48 deletions

View File

@ -31,7 +31,7 @@
const char *filename; \
int volume = 100; \
int pitch = 100; \
rb_get_args(argc, argv, "z|ii", &filename, &volume, &pitch); \
rb_get_args(argc, argv, "z|ii", &filename, &volume, &pitch, RB_ARG_END); \
GUARD_EXC( gState->audio().entity##Play(filename, volume, pitch); ) \
return Qnil; \
} \
@ -47,7 +47,7 @@ RB_METHOD(audio_##entity##Fade) \
{ \
RB_UNUSED_PARAM; \
int time; \
rb_get_args(argc, argv, "i", &time); \
rb_get_args(argc, argv, "i", &time, RB_ARG_END); \
gState->audio().bgmFade(time); \
return Qnil; \
}

View File

@ -282,10 +282,51 @@ rb_get_args(int argc, VALUE *argv, const char *format, ...)
}
}
/* Pop remaining arg pointers off
* the stack to check for RB_ARG_END */
format--;
while ((c = *format++))
{
switch (c)
{
case 'o' :
case 'S' :
va_arg(ap, VALUE*);
break;
case 's' :
va_arg(ap, const char**);
va_arg(ap, int*);
break;
case 'z' :
va_arg(ap, const char**);
break;
case 'f' :
va_arg(ap, double*);
break;
case 'i' :
va_arg(ap, int*);
break;
case 'b' :
va_arg(ap, bool*);
break;
}
}
// FIXME print num of needed args vs provided
if (!c && argc > argI)
rb_raise(rb_eArgError, "wrong number of arguments");
/* Verify correct termination */
void *argEnd = va_arg(ap, void*);
Q_UNUSED(argEnd);
Q_ASSERT(argEnd == RB_ARG_END);
va_end(ap);
return argI;

View File

@ -69,7 +69,7 @@ void initType(rb_data_type_struct &type,
void (*freeInst)(void*));
template<class C>
static inline void freeInstance(void *inst)
static void freeInstance(void *inst)
{
delete static_cast<C*>(inst);
}
@ -132,6 +132,9 @@ wrapNilProperty(VALUE self, const char *iv)
int
rb_get_args(int argc, VALUE *argv, const char *format, ...);
/* Always terminate 'rb_get_args' with this */
#define RB_ARG_END ((void*) -1)
typedef VALUE (*RubyMethod)(int argc, VALUE *argv, VALUE self);
static inline void
@ -158,7 +161,7 @@ objectLoad(int argc, VALUE *argv, VALUE self, rb_data_type_struct &type)
{
const char *data;
int dataLen;
rb_get_args(argc, argv, "s", &data, &dataLen);
rb_get_args(argc, argv, "s", &data, &dataLen, RB_ARG_END);
VALUE obj = rb_obj_alloc(self);
@ -227,7 +230,7 @@ rb_bool_new(bool value)
Klass *k = getPrivateData<Klass>(self); \
VALUE propObj; \
PropKlass *prop; \
rb_get_args(argc, argv, "o", &propObj); \
rb_get_args(argc, argv, "o", &propObj, RB_ARG_END); \
prop = getPrivateDataCheck<PropKlass>(propObj, PropKlass##Type); \
GUARD_EXC( k->set##PropName(prop); ) \
rb_iv_set(self, prop_iv, propObj); \
@ -249,7 +252,7 @@ rb_bool_new(bool value)
Klass *k = getPrivateData<Klass>(self); \
VALUE propObj; \
PropKlass *prop; \
rb_get_args(argc, argv, "o", &propObj); \
rb_get_args(argc, argv, "o", &propObj, RB_ARG_END); \
if (rb_type(propObj) == RUBY_T_NIL) \
prop = 0; \
else \
@ -271,7 +274,7 @@ rb_bool_new(bool value)
{ \
Klass *k = getPrivateData<Klass>(self); \
type value; \
rb_get_args(argc, argv, param_t_s, &value); \
rb_get_args(argc, argv, param_t_s, &value, RB_ARG_END); \
GUARD_EXC( k->set##PropName(value); ) \
return value_fun(value); \
}

View File

@ -39,14 +39,14 @@ RB_METHOD(bitmapInitialize)
if (argc == 1)
{
char *filename;
rb_get_args(argc, argv, "z", &filename);
rb_get_args(argc, argv, "z", &filename, RB_ARG_END);
GUARD_EXC( b = new Bitmap(filename); )
}
else
{
int width, height;
rb_get_args(argc, argv, "ii", &width, &height);
rb_get_args(argc, argv, "ii", &width, &height, RB_ARG_END);
GUARD_EXC( b = new Bitmap(width, height); )
}
@ -114,7 +114,7 @@ RB_METHOD(bitmapBlt)
Bitmap *src;
Rect *srcRect;
rb_get_args(argc, argv, "iioo|i", &x, &y, &srcObj, &srcRectObj, &opacity);
rb_get_args(argc, argv, "iioo|i", &x, &y, &srcObj, &srcRectObj, &opacity, RB_ARG_END);
src = getPrivateDataCheck<Bitmap>(srcObj, BitmapType);
srcRect = getPrivateDataCheck<Rect>(srcRectObj, RectType);
@ -136,7 +136,7 @@ RB_METHOD(bitmapStretchBlt)
Bitmap *src;
Rect *destRect, *srcRect;
rb_get_args(argc, argv, "ooo|i", &destRectObj, &srcObj, &srcRectObj);
rb_get_args(argc, argv, "ooo|i", &destRectObj, &srcObj, &srcRectObj, &opacity, RB_ARG_END);
src = getPrivateDataCheck<Bitmap>(srcObj, BitmapType);
destRect = getPrivateDataCheck<Rect>(destRectObj, RectType);
@ -159,7 +159,7 @@ RB_METHOD(bitmapFillRect)
VALUE rectObj;
Rect *rect;
rb_get_args(argc, argv, "oo", &rectObj, &colorObj);
rb_get_args(argc, argv, "oo", &rectObj, &colorObj, RB_ARG_END);
rect = getPrivateDataCheck<Rect>(rectObj, RectType);
color = getPrivateDataCheck<Color>(colorObj, ColorType);
@ -170,7 +170,7 @@ RB_METHOD(bitmapFillRect)
{
int x, y, width, height;
rb_get_args(argc, argv, "iiiio", &x, &y, &width, &height, &colorObj);
rb_get_args(argc, argv, "iiiio", &x, &y, &width, &height, &colorObj, RB_ARG_END);
color = getPrivateDataCheck<Color>(colorObj, ColorType);
@ -197,7 +197,7 @@ RB_METHOD(bitmapGetPixel)
int x, y;
rb_get_args(argc, argv, "ii", &x, &y);
rb_get_args(argc, argv, "ii", &x, &y, RB_ARG_END);
GUARD_EXC(
if (x < 0 || y < 0 || x >= b->width() || y >= b->height())
@ -221,7 +221,7 @@ RB_METHOD(bitmapSetPixel)
Color *color;
rb_get_args(argc, argv, "iio", &x, &y, &colorObj);
rb_get_args(argc, argv, "iio", &x, &y, &colorObj, RB_ARG_END);
color = getPrivateDataCheck<Color>(colorObj, ColorType);
@ -236,7 +236,7 @@ RB_METHOD(bitmapHueChange)
int hue;
rb_get_args(argc, argv, "i", &hue);
rb_get_args(argc, argv, "i", &hue, RB_ARG_END);
GUARD_EXC( b->hueChange(hue); );
@ -255,7 +255,7 @@ RB_METHOD(bitmapDrawText)
VALUE rectObj;
Rect *rect;
rb_get_args(argc, argv, "oz|i", &rectObj, &str, &align);
rb_get_args(argc, argv, "oz|i", &rectObj, &str, &align, RB_ARG_END);
rect = getPrivateDataCheck<Rect>(rectObj, RectType);
@ -265,7 +265,7 @@ RB_METHOD(bitmapDrawText)
{
int x, y, width, height;
rb_get_args(argc, argv, "iiiiz|i", &x, &y, &width, &height, &str, &align);
rb_get_args(argc, argv, "iiiiz|i", &x, &y, &width, &height, &str, &align, RB_ARG_END);
GUARD_EXC( b->drawText(x, y, width, height, str, align); );
}
@ -279,7 +279,7 @@ RB_METHOD(bitmapTextSize)
const char *str;
rb_get_args(argc, argv, "z", &str);
rb_get_args(argc, argv, "z", &str, RB_ARG_END);
IntRect value;
GUARD_EXC( value = b->textSize(str); );

View File

@ -40,7 +40,7 @@ DEF_TYPE(Rect);
{ \
Klass *p = getPrivateData<Klass>(self); \
arg_type arg; \
rb_get_args(argc, argv, arg_t_s, &arg); \
rb_get_args(argc, argv, arg_t_s, &arg, RB_ARG_END); \
p->set##Attr(arg); \
return *argv; \
}
@ -69,7 +69,7 @@ ATTR_INT_RW(Rect, Height)
Klass *p = getPrivateData<Klass>(self); \
VALUE otherObj; \
Klass *other; \
rb_get_args(argc, argv, "o", &otherObj); \
rb_get_args(argc, argv, "o", &otherObj, RB_ARG_END); \
other = getPrivateDataCheck<Klass>(otherObj, Klass##Type); \
return rb_bool_new(*p == *other); \
}
@ -82,7 +82,7 @@ EQUAL_FUN(Rect)
RB_METHOD(Klass##Initialize) \
{ \
param_type p1, p2, p3, p4 = last_param_def; \
rb_get_args(argc, argv, param_t_s, &p1, &p2, &p3, &p4); \
rb_get_args(argc, argv, param_t_s, &p1, &p2, &p3, &p4, RB_ARG_END); \
Klass *k = new Klass(p1, p2, p3, p4); \
setPrivateData(self, k, Klass##Type); \
return self; \
@ -96,7 +96,7 @@ INIT_FUN(Rect, int, "iiii", 0)
RB_METHOD(Klass##Set) \
{ \
param_type p1, p2, p3, p4 = last_param_def; \
rb_get_args(argc, argv, param_t_s, &p1, &p2, &p3, &p4); \
rb_get_args(argc, argv, param_t_s, &p1, &p2, &p3, &p4, RB_ARG_END); \
Klass *k = getPrivateData<Klass>(self); \
k->set(p1, p2, p3, p4); \
return self; \

View File

@ -47,7 +47,7 @@ RB_METHOD(fileIntRead)
{
int length = -1;
rb_get_args(argc, argv, "i", &length);
rb_get_args(argc, argv, "i", &length, RB_ARG_END);
SDL_RWops *ops = getPrivateData<SDL_RWops>(self);
@ -129,7 +129,7 @@ RB_METHOD(kernelLoadData)
RB_UNUSED_PARAM;
const char *filename;
rb_get_args(argc, argv, "z", &filename);
rb_get_args(argc, argv, "z", &filename, RB_ARG_END);
return kernelLoadDataInt(filename);
}
@ -141,7 +141,7 @@ RB_METHOD(kernelSaveData)
VALUE obj;
VALUE filename;
rb_get_args(argc, argv, "oS", &obj, &filename);
rb_get_args(argc, argv, "oS", &obj, &filename, RB_ARG_END);
VALUE file = rb_funcall(rb_cFile, rb_intern("open"), 2, filename, rb_str_new_cstr("w"));

View File

@ -36,7 +36,7 @@ RB_METHOD(flashableFlash)
Color *color;
rb_get_args(argc, argv, "oi", &colorObj, &duration);
rb_get_args(argc, argv, "oi", &colorObj, &duration, RB_ARG_END);
if (rb_type(colorObj) == RUBY_T_NIL)
{

View File

@ -31,7 +31,7 @@ RB_METHOD(fontDoesExist)
RB_UNUSED_PARAM;
const char *name;
rb_get_args(argc, argv, "z", &name);
rb_get_args(argc, argv, "z", &name, RB_ARG_END);
return rb_bool_new(Font::doesExist(name));
}
@ -41,7 +41,7 @@ RB_METHOD(fontInitialize)
const char *name = 0;
int size = 0;
rb_get_args(argc, argv, "|zi", &name, &size);
rb_get_args(argc, argv, "|zi", &name, &size, RB_ARG_END);
Font *f = new Font(name, size);
@ -68,7 +68,7 @@ RB_METHOD(FontSetName)
Font *f = getPrivateData<Font>(self);
VALUE name;
rb_get_args(argc, argv, "S", &name);
rb_get_args(argc, argv, "S", &name, RB_ARG_END);
f->setName(RSTRING_PTR(name));
@ -93,7 +93,7 @@ DEF_PROP_OBJ(Font, Color, Color, "color")
{ \
RB_UNUSED_PARAM; \
type value; \
rb_get_args(argc, argv, param_t_s, &value); \
rb_get_args(argc, argv, param_t_s, &value, RB_ARG_END); \
Klass::set##PropName(value); \
return value_fun(value); \
}
@ -112,7 +112,7 @@ RB_METHOD(FontSetDefaultName)
{
RB_UNUSED_PARAM;
VALUE nameObj;
rb_get_args(argc, argv, "S", &nameObj);
rb_get_args(argc, argv, "S", &nameObj, RB_ARG_END);
Font::setDefaultName(RSTRING_PTR(nameObj));
@ -129,7 +129,7 @@ RB_METHOD(FontGetDefaultColor)
RB_METHOD(FontSetDefaultColor)
{
VALUE colorObj;
rb_get_args(argc, argv, "o", &colorObj);
rb_get_args(argc, argv, "o", &colorObj, RB_ARG_END);
Color *c = getPrivateDataCheck<Color>(colorObj, ColorType);

View File

@ -50,7 +50,7 @@ RB_METHOD(graphicsTransition)
const char *filename = 0;
int vague = 40;
rb_get_args(argc, argv, "|izi", &duration, &filename, &vague);
rb_get_args(argc, argv, "|izi", &duration, &filename, &vague, RB_ARG_END);
GUARD_EXC( gState->graphics().transition(duration, filename, vague); )
@ -76,7 +76,7 @@ RB_METHOD(graphicsFrameReset)
{ \
RB_UNUSED_PARAM; \
int value; \
rb_get_args(argc, argv, "i", &value); \
rb_get_args(argc, argv, "i", &value, RB_ARG_END); \
gState->graphics().set##PropName(value); \
return rb_fix_new(value); \
}
@ -91,7 +91,7 @@ RB_METHOD(graphicsFrameReset)
{ \
RB_UNUSED_PARAM; \
bool value; \
rb_get_args(argc, argv, "b", &value); \
rb_get_args(argc, argv, "b", &value, RB_ARG_END); \
gState->graphics().set##PropName(value); \
return rb_bool_new(value); \
}

View File

@ -38,7 +38,7 @@ RB_METHOD(inputPress)
RB_UNUSED_PARAM;
int num;
rb_get_args(argc, argv, "i", &num);
rb_get_args(argc, argv, "i", &num, RB_ARG_END);
Input::ButtonCode bc = (Input::ButtonCode) num;
@ -50,7 +50,7 @@ RB_METHOD(inputTrigger)
RB_UNUSED_PARAM;
int num;
rb_get_args(argc, argv, "i", &num);
rb_get_args(argc, argv, "i", &num, RB_ARG_END);
Input::ButtonCode bc = (Input::ButtonCode) num;
@ -62,7 +62,7 @@ RB_METHOD(inputRepeat)
RB_UNUSED_PARAM;
int num;
rb_get_args(argc, argv, "i", &num);
rb_get_args(argc, argv, "i", &num, RB_ARG_END);
Input::ButtonCode bc = (Input::ButtonCode) num;

View File

@ -44,7 +44,7 @@ RB_METHOD(sceneElementSetZ)
SceneElement *se = getPrivateData<C>(self);
int z;
rb_get_args(argc, argv, "i", &z);
rb_get_args(argc, argv, "i", &z, RB_ARG_END);
GUARD_EXC( se->setZ(z); );
@ -70,7 +70,7 @@ RB_METHOD(sceneElementSetVisible)
SceneElement *se = getPrivateData<C>(self);
bool visible;
rb_get_args(argc, argv, "b", &visible);
rb_get_args(argc, argv, "b", &visible, RB_ARG_END);
GUARD_EXC( se->setVisible(visible); );

View File

@ -30,7 +30,7 @@ RB_METHOD(tableInitialize)
int x, y, z;
x = y = z = 1;
rb_get_args(argc, argv, "i|ii", &x, &y, &z);
rb_get_args(argc, argv, "i|ii", &x, &y, &z, RB_ARG_END);
Table *t = new Table(x, y, z);

View File

@ -37,7 +37,7 @@ RB_METHOD(tilemapAutotilesSet)
int i;
VALUE bitmapObj;
rb_get_args(argc, argv, "io", &i, &bitmapObj);
rb_get_args(argc, argv, "io", &i, &bitmapObj, RB_ARG_END);
Bitmap *bitmap = getPrivateDataCheck<Bitmap>(bitmapObj, BitmapType);
@ -52,7 +52,7 @@ RB_METHOD(tilemapAutotilesSet)
RB_METHOD(tilemapAutotilesGet)
{
int i;
rb_get_args (argc, argv, "i", &i);
rb_get_args (argc, argv, "i", &i, RB_ARG_END);
if (i < 0 || i > 6)
return Qnil;
@ -72,7 +72,7 @@ RB_METHOD(tilemapInitialize)
VALUE viewportObj = Qnil;
Viewport *viewport = 0;
rb_get_args(argc, argv, "|o", &viewportObj);
rb_get_args(argc, argv, "|o", &viewportObj, RB_ARG_END);
if (rb_type(viewportObj) != RUBY_T_NIL)
viewport = getPrivateDataCheck<Viewport>(viewportObj, ViewportType);

View File

@ -39,7 +39,7 @@ RB_METHOD(viewportInitialize)
VALUE rectObj;
Rect *rect;
rb_get_args(argc, argv, "o", &rectObj);
rb_get_args(argc, argv, "o", &rectObj, RB_ARG_END);
rect = getPrivateDataCheck<Rect>(rectObj, RectType);
@ -49,7 +49,7 @@ RB_METHOD(viewportInitialize)
{
int x, y, width, height;
rb_get_args(argc, argv, "iiii", &x, &y, &width, &height);
rb_get_args(argc, argv, "iiii", &x, &y, &width, &height, RB_ARG_END);
v = new Viewport(x, y, width, height);
}

View File

@ -48,7 +48,7 @@ viewportElementInitialize(int argc, VALUE *argv, VALUE self)
VALUE viewportObj = Qnil;
Viewport *viewport = 0;
rb_get_args(argc, argv, "|o", &viewportObj);
rb_get_args(argc, argv, "|o", &viewportObj, RB_ARG_END);
if (rb_type(viewportObj) != RUBY_T_NIL)
viewport = getPrivateDataCheck<Viewport>(viewportObj, ViewportType);