tvforms: Definitely fix new/delete mismatches in TDataCollection

Mismatches arose from the fact that item allocations are done from three different places (datacoll.cpp, listdlg.cpp and genform.cpp) but only genform.cpp knows the actual definition of TDataRec.

new/delete has been replaced with malloc/free in order to be consistent with the comment in listdlg.cpp that the safety pool should not be used (when building with Borland C++).

Also fix an uninitialized data access in TButton introduced by the animated key press feature (9a7bcb3).

Fixes #160.

examples/tvforms/datacoll.cpp

@@ -62,7 +62,7 @@ void *TDataCollection::readItem( ipstream& is )
 {
     void *obj;
 
-    obj = new char[itemSize];
+    obj = malloc(itemSize);
     is.readBytes(obj, itemSize);
     return obj;
 }
@@ -105,33 +105,5 @@ void TDataCollection::error( int code )
 
 void TDataCollection::freeItem( void *item )
 {
-    if (item != NULL)
-        delete[] (char *) item;
-}
-
-void TDataCollection::setLimit( int aLimit )
-{
-    void **aItems;
-
-    if (aLimit < count)
-        aLimit = count;
-    if (aLimit > maxCollectionSize)
-        aLimit = maxCollectionSize;
-    if (aLimit != limit)
-        {
-        if (aLimit == 0)
-            aItems = NULL;
-        else
-            {
-            // Restrict collection: don't allow it to eat into safety pool.
-            // Requires careful checking for success at point of insertion.
-            aItems = new void *[aLimit];
-            if ( (count != 0) && (items != 0) )
-                memcpy(aItems, items, count * sizeof(void *));
-            }
-        if (limit != 0)
-            delete[] items;
-        items = aItems;
-        limit = aLimit;
-        }
+    ::free(item);
 }

examples/tvforms/datacoll.h

@@ -27,6 +27,10 @@
 
 enum KeyTypes {stringKey, longIntKey};
 
+// WARNING: This collection can only be used with trivial types.
+// It uses malloc/free for item allocations in order to bypass the
+// safety pool when building with Borland C++.
+
 class TDataCollection : public TStringCollection
 {
 
@@ -36,7 +40,6 @@ class TDataCollection : public TStringCollection
     virtual int compare( void *, void * );
     virtual void error( int code );
     virtual void freeItem( void * );
-    virtual void setLimit( int );
 
 protected:
 

examples/tvforms/genform.cpp

@@ -67,7 +67,7 @@ int main(void)
     collection->duplicates = allowDuplicates;
     for(i = 0; i < dataCount; ++i)
         {
-        p = new TDataRec;
+        p = malloc(sizeof(TDataRec));
         memset(p, 0 , sizeof(TDataRec));   // keep padding bytes initialized
         f->setData((void *)&data[i]);      // move into object
         f->getData(p);                     // move onto heap

examples/tvforms/listdlg.cpp

@@ -477,7 +477,7 @@ Boolean TListDialog::saveForm(TDialog *f)
       return False;
 
     // Extract data from form. Don't use safety pool.
-    p = new char[dataCollection->itemSize];
+    p = malloc(dataCollection->itemSize);
     if (p == NULL)
         {
         TApplication::application->outOfMemory();
@@ -490,16 +490,16 @@ Boolean TListDialog::saveForm(TDialog *f)
     if ( (!(dataCollection->duplicates) && dataCollection->search(dataCollection->keyOf(p), i)) )
         if ( (((TForm*)f)->prevData == NULL) || (((TForm *)f)->prevData != dataCollection->at(i)) )
             {
-            delete[] (char *) p;
+            free(p);
             messageBox("Duplicate keys are not allowed in this database. "
                        "Delete duplicate record before saving this form.",
                         mfError | mfOKButton);
             return False;
             }
 
-    // Free previous data?
+    // Free previous data.
     if (((TForm *)f)->prevData != NULL)
-        dataCollection->remove(((TForm*)f)->prevData);
+        dataCollection->free(((TForm*)f)->prevData);
 
     // TDataCollection.Insert may fail because it doesn't use
     //  the safety pool. Check status field after insert and cleanup
@@ -508,7 +508,7 @@ Boolean TListDialog::saveForm(TDialog *f)
     dataCollection->insert(p);
     if (dataCollection->status != 0)
         {
-        delete[] (char *) p;
+        free(p);
         TApplication::application->outOfMemory();
         return False;
         }

source/tvision/tbutton.cpp

@@ -330,6 +330,7 @@ void *TButton::read( ipstream& is )
         state &= ~sfDisabled;
     else
         state |= sfDisabled;
+    animationTimer = 0;
     return this;
 }
 

source/tvision/tcollect.cpp

@@ -164,13 +164,9 @@ void TNSCollection::freeAll()
 
 void TNSCollection::freeItem( void *item )
 {
-    // 'delete' (which does not work on void pointers because it is unable to
-    // find a destructor function) is overriden when compiling with Borland C++
-    // (new.cpp) and has side effects.
-    // On Linux, it cannot be replaced with 'free' because mixing 'new' with 'free'
-    // or 'malloc' with 'delete' is undefined behaviour.
-    // The right thing to do is to invoke 'operator delete' directly, which
-    // deallocates memory but does not invoke any destructors.
+    // WARNING: The right thing to do is to override this method and cast the
+    // pointer to its actual type. This default implementation won't call any
+    // destructors if used with non-trivially-destructible items.
     ::operator delete(item);
 }
 

source/tvision/tobjstrm.cpp

@@ -568,7 +568,6 @@ opstream::~opstream()
 opstream& opstream::seekp( streampos pos )
 {
     objs->freeAll();
-    objs->removeAll();
 #ifdef __BORLANDC__
     bp->seekoff( pos, ios::beg );
 #else
@@ -580,7 +579,6 @@ opstream& opstream::seekp( streampos pos )
 opstream& opstream::seekp( streamoff pos, pstream::seekdir dir )
 {
     objs->freeAll();
-    objs->removeAll();
 #ifdef __BORLANDC__
     bp->seekoff( pos, ::seekdir(dir) );
 #else