Replace DATA_PTR and Data_Get_Struct by SWIG_ConvertPtr

Mixing ruby functions and SWIG functions to access Ruby/C++ objects is bad practice.
It relies on a particular expectation of how SWIG maps C++ objects to Ruby objects.

This commit changes the code to use SWIG functions only and to not use raw Ruby functions to access SWIG mapped objects.

Also change the flag of ConvertPtr to the SWIG provided constant SWIG_POINTER_DISOWN, to make it more clear, that the object will be deleted by the object it was assigned to.

The compiler deprecation warning is re-enabled, since they do no longer appear when built with SWIG with the following patch:
  swig/swig#3326

Fix type conversions of Array<FXPoint>, Array<FXRectangle>, Array<FXSegment> and Array<FXArc> to raise an error on type mismatch.
This wasn't checked before and could lead to invalid memory access.

Author: larskanis

ext/fox16_c/FXRbApp.cpp

@@ -24,6 +24,7 @@
  * $Id: FXRbApp.cpp 2902 2008-12-11 14:09:20Z lyle $
  ***********************************************************************/
 
+#include "swigruby.h"
 #include "FXRbCommon.h"
 
 #ifdef HAVE_SYS_TIME_H

ext/fox16_c/FXRbDataTarget.cpp

@@ -24,6 +24,7 @@
  * $Id: FXRbDataTarget.cpp 2713 2007-11-14 15:27:36Z lyle $
  ***********************************************************************/
 
+#include "swigruby.h"
 #include "FXRbCommon.h"
 
 /**

ext/fox16_c/FXRbGLViewer.cpp

@@ -24,6 +24,7 @@
  * $Id: FXRbGLViewer.cpp 2190 2005-08-24 07:58:47Z lyle $
  ***********************************************************************/
 
+#include "swigruby.h"
 #include "FXRbCommon.h"
 
 // Process picks

ext/fox16_c/FXRbObjRegistry.cpp

@@ -20,9 +20,9 @@
  * at "lars@greiz-reinsdorf.de".
  ***********************************************************************/
 
+#include "swigruby.h"
 #include "FXRbCommon.h"
 #include "FXRbObjRegistry.h"
-#include "swigruby.h"
 
 FXRbObjRegistry::FXRbObjRegistry(){
   FXRuby_Objects=st_init_numtable();
@@ -84,7 +84,7 @@ void FXRbObjRegistry::RegisterRubyObj(VALUE rubyObj,const void* foxObj) {
      * To avoid double references to the same foxObj from different Ruby objects,
      * we decouple the foxObj from previoius ruby object and point to the new one.
      */
-    DATA_PTR(desc->obj) = 0;
+    FXRbConvertPtr(desc->obj, NULL, SWIG_POINTER_RELEASE);
     desc->obj = rubyObj;
     desc->type = own;
   } else {
@@ -107,7 +107,14 @@ void FXRbObjRegistry::UnregisterRubyObj(const void* foxObj, bool alsoOwned){
     if(st_lookup(FXRuby_Objects,reinterpret_cast<st_data_t>(const_cast<void*>(foxObj)),reinterpret_cast<st_data_t *>(&desc))!=0){
       if( !alsoOwned && desc->type!=borrowed ) return;
       FXTRACE((1,"FXRbUnregisterRubyObj(rubyObj=%p (%s),foxObj=%p)\n",(void *)desc->obj,safe_rb_obj_classname(desc->obj),foxObj));
-      DATA_PTR(desc->obj)=0;
+
+      if(RB_TYPE_P(desc->obj, RUBY_T_DATA)) {
+        /* Release unless it's already T_ZOMBIE */
+        int res = SWIG_ConvertPtr(desc->obj, NULL, NULL, SWIG_POINTER_RELEASE);
+        if (res != SWIG_OK){
+          rb_bug( "UnregisterRubyObj(rubyObj=%p) error: %d", (void*)desc->obj, res);
+        }
+      }
       FXFREE(&desc);
       st_delete(FXRuby_Objects,reinterpret_cast<st_data_t *>(const_cast<void**>(&foxObj)),reinterpret_cast<st_data_t *>(0));
       FXASSERT(st_lookup(FXRuby_Objects,reinterpret_cast<st_data_t>(const_cast<void*>(foxObj)),reinterpret_cast<st_data_t *>(0))==0);

ext/fox16_c/FXRuby.cpp

@@ -28,13 +28,13 @@
 #pragma warning (disable : 4786)
 #endif
 
+// SWIG runtime functions we need
+#include "swigruby.h"
+
 #include "FXRbCommon.h"
 #include "FXRbObjRegistry.h"
 #include "impl.h"
 
-// SWIG runtime functions we need
-#include "swigruby.h"
-
 #ifdef __CYGWIN__
 #include <io.h>		// for get_osf_handle()
 #endif
@@ -842,8 +842,7 @@ void* FXRbGetExpectedData(VALUE recv,FXSelector key,VALUE value){
   FXushort id=FXSELID(key);
 
   // Extract the FOX object (the receiver) from this Ruby instance
-  FXObject* obj;
-  Data_Get_Struct(recv,FXObject,obj);
+  FXObject *obj = (FXObject*)FXRbConvertPtr(recv, NULL, 0);
 
   FXASSERT(type!=SEL_NONE);
   FXASSERT(type!=SEL_LAST);
@@ -896,7 +895,7 @@ void* FXRbGetExpectedData(VALUE recv,FXSelector key,VALUE value){
     case SEL_DND_MOTION:
     case SEL_DND_REQUEST:
     case SEL_PICKED:
-      SWIG_ConvertPtr(value,&ptr,FXRbTypeQuery("FXEvent *"),1);
+      SWIG_ConvertPtr(value,&ptr,FXRbTypeQuery("FXEvent *"),SWIG_POINTER_DISOWN);
       return ptr;
     case SEL_IO_READ:
     case SEL_IO_WRITE:
@@ -1072,7 +1071,7 @@ void* FXRbGetExpectedData(VALUE recv,FXSelector key,VALUE value){
 
   if(type==SEL_CHANGED){
 	  if(obj->isMemberOf(FXMETACLASS(FXPicker))){
-			SWIG_ConvertPtr(value,&ptr,FXRbTypeQuery("FXPoint *"),1);
+			SWIG_ConvertPtr(value,&ptr,FXRbTypeQuery("FXPoint *"),SWIG_POINTER_DISOWN);
 			return ptr;
     }
     if(obj->isMemberOf(FXMETACLASS(FXWindow))){
@@ -1086,7 +1085,7 @@ void* FXRbGetExpectedData(VALUE recv,FXSelector key,VALUE value){
   }
 
 	if(type==SEL_DRAGGED){
-	    SWIG_ConvertPtr(value,&ptr,FXRbTypeQuery("FXEvent *"),1);
+	    SWIG_ConvertPtr(value,&ptr,FXRbTypeQuery("FXEvent *"),SWIG_POINTER_DISOWN);
 	    return ptr;
 	    }
 
@@ -1300,21 +1299,21 @@ FXGLObject* FXRbCallGLObjectMethod_gvlcb(FXGLObject* recv,const char *func){
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),0);
-  return NIL_P(result) ? 0 : reinterpret_cast<FXGLObject*>(DATA_PTR(result));
+  return (FXGLObject*)FXRbConvertPtr(result, NULL, 0);
   }
 
 FXGLObject* FXRbCallGLObjectMethod_gvlcb(FXGLViewer* recv,const char *func,FXint x,FXint y){
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),2,INT2NUM(x),INT2NUM(y));
-  return NIL_P(result) ? 0 : reinterpret_cast<FXGLObject*>(DATA_PTR(result));
+  return (FXGLObject*)FXRbConvertPtr(result, NULL, 0);
   }
 
 FXGLObject* FXRbCallGLObjectMethod_gvlcb(FXGLObject* recv,const char *func,FXuint* path,FXint n){
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),1,FXRbMakeArray(path,n));
-  return NIL_P(result) ? 0 : reinterpret_cast<FXGLObject*>(DATA_PTR(result));
+  return (FXGLObject*)FXRbConvertPtr(result, NULL, 0);
   }
 
 //----------------------------------------------------------------------
@@ -1329,7 +1328,8 @@ FXGLObject** FXRbCallGLObjectArrayMethod_gvlcb(FXGLViewer* recv,const char *func
     Check_Type(result,T_ARRAY);
     if(FXMALLOC(&objects,FXGLObject*,RARRAY_LEN(result)+1)){
       for(long i=0; i<RARRAY_LEN(result); i++){
-	objects[i]=reinterpret_cast<FXGLObject*>(DATA_PTR(rb_ary_entry(result,i)));
+        VALUE entry = rb_ary_entry(result,i);
+        objects[i]=(FXGLObject*)FXRbConvertPtr(entry, NULL, 0);
         }
       objects[RARRAY_LEN(result)]=0;
       }
@@ -1344,14 +1344,14 @@ FXTableItem* FXRbCallTableItemMethod_gvlcb(FXTable* recv,const char *func,const
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),3,to_ruby(text),to_ruby_cb(icon),itemData);
-  return NIL_P(result)?0:reinterpret_cast<FXTableItem*>(DATA_PTR(result));
+  return (FXTableItem*)FXRbConvertPtr(result, NULL, 0);
   }
 
 FXTableItem* FXRbCallTableItemMethod_gvlcb(FXTable* recv,const char *func,FXint row,FXint col,FXbool notify){
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),3,to_ruby(row),to_ruby(col),to_ruby(notify));
-  return NIL_P(result)?0:reinterpret_cast<FXTableItem*>(DATA_PTR(result));
+  return (FXTableItem*)FXRbConvertPtr(result, NULL, 0);
   }
 
 //----------------------------------------------------------------------
@@ -1360,7 +1360,7 @@ FXTreeItem* FXRbCallTreeItemMethod_gvlcb(const FXTreeList* recv,const char *func
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),2,INT2NUM(x),INT2NUM(y));
-  return NIL_P(result) ? 0 : reinterpret_cast<FXTreeItem*>(DATA_PTR(result));
+  return (FXTreeItem*)FXRbConvertPtr(result, NULL, 0);
   }
 
 //----------------------------------------------------------------------
@@ -1369,7 +1369,7 @@ FXFoldingItem* FXRbCallFoldingItemMethod_gvlcb(const FXFoldingList* recv,const c
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),2,INT2NUM(x),INT2NUM(y));
-  return NIL_P(result) ? 0 : reinterpret_cast<FXFoldingItem*>(DATA_PTR(result));
+  return (FXFoldingItem*)FXRbConvertPtr(result, NULL, 0);
   }
 
 //----------------------------------------------------------------------
@@ -1378,7 +1378,7 @@ FXFileAssoc* FXRbCallFileAssocMethod_gvlcb(const FXFileDict* recv,const char *fu
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),1,to_ruby(pathname));
-  return NIL_P(result) ? 0 : reinterpret_cast<FXFileAssoc*>(DATA_PTR(result));
+  return (FXFileAssoc*)FXRbConvertPtr(result, NULL, 0);
   }
 
 //----------------------------------------------------------------------
@@ -1388,7 +1388,7 @@ FXIcon* FXRbCallIconMethod_gvlcb(const FXTableItem* recv,const char *func){
   FXASSERT(!NIL_P(obj));
 	if(!NIL_P(obj)){
 	  VALUE result=rb_funcall(obj,rb_intern(func),0);
-	  return NIL_P(result) ? 0 : reinterpret_cast<FXIcon*>(DATA_PTR(result));
+	  return (FXIcon*)FXRbConvertPtr(result, NULL, 0);
 		}
 	else{
 		return 0;
@@ -1401,7 +1401,7 @@ FXWindow* FXRbCallWindowMethod_gvlcb(const FXTableItem* recv,const char *func,FX
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),1,to_ruby_cb(table));
-  return NIL_P(result) ? 0 : reinterpret_cast<FXWindow*>(DATA_PTR(result));
+  return (FXWindow*)FXRbConvertPtr(result, NULL, 0);
   }
 
 //----------------------------------------------------------------------
@@ -1411,7 +1411,7 @@ FXRangef FXRbCallRangeMethod_gvlcb(FXObject* recv,const char *func){
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),0);
-  return *reinterpret_cast<FXRangef*>(DATA_PTR(result));
+  return *(FXRangef*)FXRbConvertPtr(result, NULL, 0);
   }
 
 //----------------------------------------------------------------------
@@ -1606,11 +1606,34 @@ FXbool FXRbGLViewer::sortProc(FXfloat*& buffer,FXint& used,FXint& size){
  * FXRbConvertPtr() is just a wrapper around SWIG_ConvertPtr().
  */
 
-void* FXRbConvertPtr(VALUE obj,swig_type_info* ty){
+void* FXRbConvertPtr(VALUE obj,swig_type_info* ty, int flags){
   void *ptr;
-  SWIG_ConvertPtr(obj,&ptr,ty,1);
-  return ptr;
+  int res = SWIG_ConvertPtr(obj,&ptr,ty,flags);
+  if( res == SWIG_OK ) return ptr;
+#ifdef HAVE_RB_DURING_GC
+  if( rb_during_gc() ){
+    rb_bug( "FXRbConvertPtr got wrong argument type rubyObj=%p", (void*)obj);
   }
+#endif
+  if( res == SWIG_ERROR_RELEASE_NOT_OWNED ){
+    rb_raise( rb_eTypeError, "clean and disown of non-owned object is not allowed: %" PRIsVALUE, obj);
+  }
+  if( res == SWIG_NullReferenceError ){
+    rb_raise( rb_eTypeError, "object can not be NULL: %" PRIsVALUE, obj);
+  }
+  if( res == SWIG_ObjectPreviouslyDeletedError ){
+    if(ty){
+      rb_raise( rb_eTypeError, "the object has already been deleted: %" PRIsVALUE, ((swig_class *) (ty->clientdata))->klass);
+    } else {
+      rb_raise( rb_eTypeError, "the object has already been deleted");
+    }
+  }
+  if(ty){
+    rb_raise( rb_eTypeError, "wrong argument type %" PRIsVALUE ", expected kind of %" PRIsVALUE, rb_obj_class(obj), ((swig_class *) (ty->clientdata))->klass );
+  } else {
+    rb_raise( rb_eTypeError, "wrong argument type %" PRIsVALUE, rb_obj_class(obj) );
+  }
+}
 
 
 // Returns an FXInputHandle for this Ruby file object

ext/fox16_c/extconf.rb

@@ -366,7 +366,7 @@ def install
 if RbConfig::MAKEFILE_CONFIG['CC'] =~ /gcc/
   $CXXFLAGS += " -Wno-unused-function"
   $CXXFLAGS += " -Wno-maybe-uninitialized"
-  $CXXFLAGS += " -Wno-attribute-warning -Wno-deprecated-declarations"
+  $CXXFLAGS += " -Wno-attribute-warning"
 end
 
 # Last step: build the makefile

ext/fox16_c/gvl_wrappers.cpp

@@ -3,6 +3,7 @@
  *
  */
 
+#include "swigruby.h"
 #include "FXRbCommon.h"
 
 #ifdef HAVE___THREAD

ext/fox16_c/impl.cpp

@@ -1,3 +1,4 @@
+#include "swigruby.h"
 #include "FXRbCommon.h"
 /* Start stub implementations for class FXMemoryBuffer */
 

ext/fox16_c/include/FXRbDCWindow.h

@@ -44,7 +44,7 @@ class FXRbDCWindow : public FXDCWindow {
 
   // Helper for FXDCWindow initialization block
   static VALUE endit(VALUE obj){
-    FXDCWindow* dc=reinterpret_cast<FXDCWindow*>(DATA_PTR(obj));
+    FXDCWindow* dc=(FXDCWindow*)FXRbConvertPtr(obj, NULL, 0);
     FXASSERT(dc!=0);
     dc->end();
     return Qnil;

ext/fox16_c/include/FXRuby.h

@@ -48,7 +48,7 @@ static int FXSWIG_ConvertPtr(VALUE obj, void **ptr, swig_type_info *ty, int flag
 template <class TYPE>
 VALUE showHelper(VALUE self, int argc, VALUE *argv, TYPE *p, swig_type_info *typeinfo) {
   TYPE *win;
-  FXSWIG_ConvertPtr(self,(void**)&win,typeinfo,1);
+  FXSWIG_ConvertPtr(self,(void**)&win,typeinfo,SWIG_POINTER_DISOWN);
   if (argc == 0) {
     win->_show();
     }
@@ -73,7 +73,7 @@ bool FXRbIsInGC(const void* ptr);
 swig_type_info *FXRbTypeQuery(const char *name);
 
 // Wrapper around SWIG_ConvertPtr()
-void* FXRbConvertPtr(VALUE obj,swig_type_info* typeinfo);
+void* FXRbConvertPtr(VALUE obj,swig_type_info* typeinfo, int flags);
 
 // Returns an FXInputHandle for this Ruby file object
 FXInputHandle FXRbGetReadFileHandle(VALUE obj,FXuint mode);
@@ -694,15 +694,15 @@ FXIcon* FXRbCallIconMethod_gvlcb(const FXIconSource *recv,const char *func,TYPE1
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),2,to_ruby(arg1),to_ruby(arg2));
-  return NIL_P(result) ? 0 : reinterpret_cast<FXIcon*>(DATA_PTR(result));
+  return (FXIcon*)FXRbConvertPtr(result, NULL, 0);
   }
 
 template<class TYPE1, class TYPE2, class TYPE3, class TYPE4>
 FXIcon* FXRbCallIconMethod_gvlcb(const FXIconSource *recv,const char *func,TYPE1& arg1,TYPE2 arg2,TYPE3 arg3,const TYPE4& arg4){
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),4,to_ruby(arg1),to_ruby(arg2),to_ruby(arg3),to_ruby(arg4));
-  return NIL_P(result) ? 0 : reinterpret_cast<FXIcon*>(DATA_PTR(result));
+  return (FXIcon*)FXRbConvertPtr(result, NULL, 0);
   }
 
 // Call functions with FXImage* return value
@@ -711,15 +711,15 @@ FXImage* FXRbCallImageMethod_gvlcb(const FXIconSource *recv,const char *func,TYP
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),2,to_ruby(arg1),to_ruby(arg2));
-  return NIL_P(result) ? 0 : reinterpret_cast<FXImage*>(DATA_PTR(result));
+  return (FXImage*)FXRbConvertPtr(result, NULL, 0);
   }
 
 template<class TYPE1, class TYPE2, class TYPE3, class TYPE4>
 FXImage* FXRbCallImageMethod_gvlcb(const FXIconSource *recv,const char *func,TYPE1& arg1,TYPE2 arg2,TYPE3 arg3,const TYPE4& arg4){
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
   VALUE result=rb_funcall(obj,rb_intern(func),4,to_ruby(arg1),to_ruby(arg2),to_ruby(arg3),to_ruby(arg4));
-  return NIL_P(result) ? 0 : reinterpret_cast<FXImage*>(DATA_PTR(result));
+  return (FXImage*)FXRbConvertPtr(result, NULL, 0);
   }
 
 // Call functions with "FXWindow*" return value