[MOAB-dev] On type correctness

Jed Brown jed at 59A2.org
Tue Sep 9 06:45:16 CDT 2008


The attached patch uses incomplete types for type-checking in the iBase,
iMesh, and (what there is of) iMeshP.  It is binary compatible with
previous versions which used void*.  There are two functions in iMesh.h
which had incorrect types (iMesh_getAdjEntities and iMesh_addEntSet) so
these should certainly be fixed globally.  There were several places in
iMesh_MOAB.cpp, iMeshP_MOAB.cpp, and testc_cbind.c where the stated
types were incorrect with regard to iBase_EntityHandle versus
iBase_EntitySetHandle.

There is one test (entityset_tag_delete_test) which fails for some
meshes.  The test is brittle since it really needs an explicit
precondition that the first set has at least two tags.  I think
iMesh_destroyTag should return iBase_TAG_IN_USE rather than
iBase_FAILURE when the tag cannot be removed.


A minor bug: since the MOAB default ordering is iBase_INTERLEAVED,
iMesh_getDfltStorage() should probably not return iBase_BLOCKED.
(Included in the patch.)


iMesh interface question: What is the expected use case with tags of
length greater than 1?  It seems to be that tags of any type other than
iBase_BYTES are not really expected to have length > 1, at least it is
not possible to put such tags on an EntitySet.  When tagging other
Entities, iMesh_setXXXArrData() works fine for larger tags.


If I insert more than one of a particular entity (like an edge or face),
I still get a status of iBase_NEW, but then when I ask for adjacency,
I get this:

  iBase_NOT_SUPPORTED: iMesh_getEntArrAdj: trouble getting adjacency list.

Not a surprise, but it's probably better to fail at insertion time.  Is
that too expensive for some data models?  Should the standard specify
the behavior in this case or is it implementation dependent?


Jed
-------------- next part --------------
Index: tools/iMesh/iMesh_MOAB.cpp
===================================================================
--- tools/iMesh/iMesh_MOAB.cpp	(revision 2057)
+++ tools/iMesh/iMesh_MOAB.cpp	(working copy)
@@ -302,7 +302,7 @@
   }
    
   void iMesh_load(iMesh_Instance instance,
-                  const iBase_EntityHandle handle,
+                  const iBase_EntitySetHandle handle,
                   const char *name, const char *options, 
                   int *err, int name_len, int options_len) 
   {
@@ -337,7 +337,7 @@
   }
 
   void iMesh_save(iMesh_Instance instance,
-                  const iBase_EntityHandle handle,
+                  const iBase_EntitySetHandle handle,
                   const char *name, const char *options, 
                   int *err, const int name_len, int options_len) 
   {
@@ -378,7 +378,7 @@
   void iMesh_getDfltStorage(iMesh_Instance instance,
                             int *order, int *err)
   {
-    *order = iBase_BLOCKED;
+    *order = iBase_INTERLEAVED;
     RETURN(iBase_SUCCESS);
   }
   
@@ -830,13 +830,13 @@
                              /*in*/ const int requested_entity_type,
                              /*in*/ const int requested_entity_topology,
                              /*in*/ const int requested_array_size,
-                             /*out*/ iMesh_EntityIterator* entArr_iterator,
+                             /*out*/ iMesh_EntityArrIterator* entArr_iterator,
                              int *err) 
   {
     MBEntityType req_type = mb_topology_table[requested_entity_topology];
     int req_dimension = (req_type == MBMAXTYPE ? (int) requested_entity_type : -1);
     RangeIterator *new_it = new RangeIterator;
-    *entArr_iterator = reinterpret_cast<iMesh_EntityIterator>(new_it);
+    *entArr_iterator = reinterpret_cast<iMesh_EntityArrIterator>(new_it);
     new_it->requestedSize = requested_array_size;
   
     MBErrorCode result;
@@ -874,7 +874,7 @@
  * Method:  getEntArrNextIter[]
  */
   void iMesh_getNextEntArrIter (iMesh_Instance instance,
-                                /*in*/ iMesh_EntityIterator entArr_iterator,
+                                /*in*/ iMesh_EntityArrIterator entArr_iterator,
                                 /*inout*/ iBase_EntityHandle** entity_handles,
                                 /*inout*/ int* entity_handles_allocated,
                                 /*out*/ int* entity_handles_size,
@@ -910,7 +910,7 @@
  * Method:  resetEntArrIter[]
  */
   void iMesh_resetEntArrIter (iMesh_Instance instance,
-                              /*in*/ iMesh_EntityIterator entArr_iterator, int *err) 
+                              /*in*/ iMesh_EntityArrIterator entArr_iterator, int *err) 
   {
     RangeIterator *this_it = RANGE_ITERATOR(entArr_iterator);
 
@@ -920,7 +920,7 @@
   }
 
   void iMesh_endEntArrIter (iMesh_Instance instance,
-                            /*in*/ iMesh_EntityIterator entArr_iterator, int *err) 
+                            /*in*/ iMesh_EntityArrIterator entArr_iterator, int *err) 
   {
     RangeIterator *this_it = RANGE_ITERATOR(entArr_iterator);
 
@@ -1095,7 +1095,7 @@
     }
   
       // return EntitySet_Handle
-    *entity_set_created = (iBase_EntityHandle)meshset;
+    *entity_set_created = (iBase_EntitySetHandle)meshset;
     RETURN(iBase_ERROR_MAP[result]);
   }
 
@@ -1178,7 +1178,7 @@
     int k = 0;
 
     for (; iter != end_iter; iter++)
-      (*contained_entset_handles)[k++] = (iBase_EntityHandle)*iter;
+      (*contained_entset_handles)[k++] = (iBase_EntitySetHandle)*iter;
 
     *contained_entset_handles_size = sets.size();
     RETURN(iBase_SUCCESS);
@@ -2431,7 +2431,7 @@
                           int *err) 
   {
     iMesh_initEntArrIter(instance, entity_set_handle, requested_entity_type,
-                         requested_entity_topology, 1, entity_iterator,
+                         requested_entity_topology, 1, (iMesh_EntityArrIterator*)entity_iterator,
                          err);
   }
 
@@ -2442,20 +2442,20 @@
   {
     int eh_size = 1;
     iMesh_getNextEntArrIter(instance,
-                            entity_iterator, &entity_handle, &eh_size, &eh_size, is_end, err);
+                            (iMesh_EntityArrIterator)entity_iterator, &entity_handle, &eh_size, &eh_size, is_end, err);
   
   }
 
   void iMesh_resetEntIter (iMesh_Instance instance,
                            /*in*/ iMesh_EntityIterator entity_iterator, int *err) 
   {
-    iMesh_resetEntArrIter(instance, entity_iterator, err);  
+    iMesh_resetEntArrIter(instance, (iMesh_EntityArrIterator)entity_iterator, err);  
   }
 
   void iMesh_endEntIter (iMesh_Instance instance,
                          /*in*/ iMesh_EntityIterator entity_iterator, int *err) 
   {
-    iMesh_endEntArrIter(instance, entity_iterator, err);
+    iMesh_endEntArrIter(instance, (iMesh_EntityArrIterator)entity_iterator, err);
   }
 
   void iMesh_getEntTopo (iMesh_Instance instance,
Index: tools/iMesh/iMesh.h
===================================================================
--- tools/iMesh/iMesh.h	(revision 2057)
+++ tools/iMesh/iMesh.h	(working copy)
@@ -76,19 +76,19 @@
      *
      * Type used to store iMesh interface handle
      */
-  typedef void* iMesh_Instance;
+  typedef struct iMesh_Instance_private *iMesh_Instance;
 
     /**\brief  Type used to store an iterator returned by iMesh
      *
      * Type used to store an iterator returned by iMesh
      */
-  typedef void* iMesh_EntityIterator;
+  typedef struct iMesh_EntityIterator_private *iMesh_EntityIterator;
 
     /**\brief  Type used to store an array iterator returned by iMesh
      *
      * Type used to store an array iterator returned by iMesh
      */
-  typedef void* iMesh_EntityArrIterator;
+  typedef struct iMesh_EntityArrIterator_private *iMesh_EntityArrIterator;
 
     /**\brief  Enumerator specifying entity topology
      *
@@ -514,7 +514,7 @@
      * \param *err Pointer to error type returned from function
      */
   void iMesh_getAdjEntities(iMesh_Instance instance,
-                            /*in*/ const iBase_EntityHandle entity_set_handle,
+                            /*in*/ const iBase_EntitySetHandle entity_set_handle,
                             /*in*/ const int entity_type_requestor,
                             /*in*/ const int entity_topology_requestor,
                             /*in*/ const int entity_type_requested,
@@ -870,7 +870,7 @@
      * \param *err Pointer to error type returned from function
      */
   void iMesh_addEntSet(iMesh_Instance instance,
-                       /*in*/ const iBase_EntityHandle entity_set_to_add,
+                       /*in*/ const iBase_EntitySetHandle entity_set_to_add,
                        /*inout*/ iBase_EntitySetHandle* entity_set_handle,
                        /*out*/ int *err);
 
Index: tools/iMesh/testc_cbind.c
===================================================================
--- tools/iMesh/testc_cbind.c	(revision 2057)
+++ tools/iMesh/testc_cbind.c	(working copy)
@@ -796,7 +796,7 @@
   }
 
     // check if parent is really added
-  iBase_EntityHandle *parents = NULL;
+  iBase_EntitySetHandle *parents = NULL;
   int parents_alloc = 0, parents_size;
   iMesh_getPrnts(mesh, parent_child, 0, 
                  &parents, &parents_alloc, &parents_size, &result);
@@ -1636,7 +1636,7 @@
 }
 
 int entityset_struct_tag_test(iMesh_Instance mesh, 
-                               iBase_EntityHandle *sets, int sets_size,
+                               iBase_EntitySetHandle *sets, int sets_size,
                                iBase_TagHandle *struct_tag) 
 {
   int result;
@@ -1692,7 +1692,7 @@
 }
 
 int entityset_tag_delete_test(iMesh_Instance mesh, 
-                               iBase_EntityHandle *sets, int sets_size) 
+                               iBase_EntitySetHandle *sets, int sets_size) 
 {
     // test forced, unforced deletion of tags from entities
   int result;
@@ -1700,7 +1700,7 @@
 
     // test getAlliBase_TagHandles for first entity
   iBase_TagHandle *all_tags = NULL;
-  iBase_EntityHandle dum_entity = sets[0];
+  iBase_EntitySetHandle dum_entity = sets[0];
   int all_tags_alloc = 0, all_tags_size;
   iMesh_getAllEntSetTags(mesh, sets[0], &all_tags, &all_tags_alloc,
 			 &all_tags_size, &result);
Index: tools/iMesh/iMeshP_MOAB.cpp
===================================================================
--- tools/iMesh/iMeshP_MOAB.cpp	(revision 2057)
+++ tools/iMesh/iMeshP_MOAB.cpp	(working copy)
@@ -51,6 +51,8 @@
 #define CONST_ENTITY_HANDLE(handle) reinterpret_cast<const MBEntityHandle>(handle)
 #define RANGE_ITERATOR(it) reinterpret_cast<RangeIterator*>(it)
 #define CAST_TO_VOID(ptr) reinterpret_cast<void*>(ptr)
+#define CAST_TO_PART_HANDLE(ptr) reinterpret_cast<iMeshP_PartHandle>(ptr)
+#define CAST_TO_ENTITY_HANDLE(ptr) reinterpret_cast<iBase_EntityHandle>(ptr)
 
 const iBase_ErrorType iBase_ERROR_MAP[] = 
 {
@@ -150,7 +152,7 @@
     int i;
     for (i = 0, rit = pc->partition_sets().begin(); 
          rit != pc->partition_sets().end(); rit++, i++)
-      (*part_handles)[i] = CAST_TO_VOID(*rit);
+      (*part_handles)[i] = CAST_TO_PART_HANDLE(*rit);
   
     RETURN(iBase_SUCCESS);
   }
@@ -304,7 +306,7 @@
       CHECK_SIZE(*copies_entity_handles, *copies_entity_handles_allocated, 
                  *copies_entity_handles_size, iBase_EntityHandle, );
       (*part_handles)[0] = 0;
-      (*copies_entity_handles)[0] = CAST_TO_VOID(shared_handle);
+      (*copies_entity_handles)[0] = CAST_TO_ENTITY_HANDLE(shared_handle);
       RETURN(iBase_SUCCESS);
     }
   
@@ -331,10 +333,10 @@
     CHECK_SIZE(*part_handles, *part_handles_allocated, *part_handles_size, 
                iMeshP_PartHandle, );
     CHECK_SIZE(*copies_entity_handles, *copies_entity_handles_allocated, 
-               *copies_entity_handles_size, iMeshP_PartHandle, );
+               *copies_entity_handles_size, iBase_EntityHandle, );
     std::copy(&shared_handles[0], &shared_handles[index], 
               HANDLE_ARRAY_PTR(*copies_entity_handles));
-    std::fill(*part_handles, *part_handles+index, CAST_TO_VOID(0));
+    std::fill(*part_handles, *part_handles+index, CAST_TO_PART_HANDLE(0));
 
     RETURN(iBase_SUCCESS);
   }
Index: tools/iMesh/iBase.h
===================================================================
--- tools/iMesh/iBase.h	(revision 2057)
+++ tools/iMesh/iBase.h	(working copy)
@@ -15,10 +15,10 @@
      * TYPEDEF'S
      *==========================================================
      */
-  typedef void* iBase_Instance;
-  typedef void* iBase_EntityHandle;
-  typedef void* iBase_EntitySetHandle;
-  typedef void* iBase_TagHandle;
+  typedef struct iBase_Instance_private        *iBase_Instance;
+  typedef struct iBase_EntityHandle_private    *iBase_EntityHandle;
+  typedef struct iBase_EntitySetHandle_private *iBase_EntitySetHandle;
+  typedef struct iBase_TagHandle_private       *iBase_TagHandle;
 
     /*==========================================================
      * ENTITYTYPE ENUMERATION
Index: tools/iMesh/iMeshP.h
===================================================================
--- tools/iMesh/iMeshP.h	(revision 2057)
+++ tools/iMesh/iMeshP.h	(working copy)
@@ -20,13 +20,13 @@
      *
      * Type used to store a part handle
      */
-  typedef void* iMeshP_PartHandle;
+  typedef struct iMeshP_PartHandle_private *iMeshP_PartHandle;
 
     /**\brief  Type used to store a partition handle
      *
      * Type used to store a partition handle
      */
-  typedef void* iMeshP_PartitionHandle;
+  typedef struct iMeshP_PartitionHandle_private *iMeshP_PartitionHandle;
 
   const int iMeshP_INTERNAL = 0;
   const int iMeshP_BOUNDARY = 1;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <https://lists.mcs.anl.gov/mailman/private/moab-dev/attachments/20080909/abd8ba60/attachment.pgp>


More information about the moab-dev mailing list