Re: Patch to convert assert statements in Engine to exceptions when available

[David Sugar]

The engine code was all done by Daniel originally.  So it has a
different style than the rest.  I think, in theory, for consistency,
engine should be changed to use tabs.  In any case, I was able to apply
this patch successfully.  I like the idea of expanding THROW to post
error messages on systems without exceptions, and I will look at that
for 1.3.  Perhaps it should invoke slog to log error strings rather 
than cerr?  Since they "abort", they probably should be logged as 
CRIT level...

Chad Yates wrote:

> Attached is a patch.
> 
>  - removed unnecessary Engine mode asserts statements
>  - converted assert statements in readBinary/writeBinary to exceptions 
> using the THROW macro. these should be the only necessary checkpoints as 
> all reads end in calls to these to do the actual i/o.
>  - ran expand -t 2 on it to clean up inconsistent tab vs 2 space 
> indentation.  what is the standard anyway? tabs or spaces?
> 
> It would be nice to change the THROW macro to output the string in the 
> case of a system with no exceptions:
> 
> from:
>  #define THROW(x) abort()
> to:
>  #define THROW(x) std::cerr << x << std::endl; abort();
> 
> But I don't know if that would negatively affect any "non-string" calls, 
> assuming there are any.
> 
> ,Chad

> 45c45
> < #if !defined(_MSC_VER) || _MSC_VER >= 1300
> ---
> > #if !defined(_MSC_VER) || _MSC_VER >= 1300
> 48a49
> > #ifndef HAVE_EXCEPTION
> 49a51
> > #endif
> 51c53
> < #ifdef      CCXX_NAMESPACES
> ---
> > #ifdef  CCXX_NAMESPACES
> 60c62
> < #ifdef      CCXX_EXCEPTIONS
> ---
> > #ifdef  CCXX_EXCEPTIONS
> 62c64
> <     PersistException(reason) 
> ---
> >   PersistException(reason) 
> 77c79
> <     : myUnderlyingStream(stream), myOperationalMode(mode)
> ---
> >   : myUnderlyingStream(stream), myOperationalMode(mode)
> 88,97c90,99
> <     {
> <       myZStream.next_in = myCompressedDataBuffer;
> <       myZStream.next_out = myUncompressedDataBuffer;
> <       myZStream.avail_in = 0;
> <       myZStream.avail_out = MAX_BUFFER;
> <       int err = inflateInit(&myZStream);
> <       if (err != Z_OK) {
> <               THROW (PersistException(String("zLib didn't initialise for 
> inflating.")));
> <       }
> <     }
> ---
> >   {
> >     myZStream.next_in = myCompressedDataBuffer;
> >     myZStream.next_out = myUncompressedDataBuffer;
> >     myZStream.avail_in = 0;
> >     myZStream.avail_out = MAX_BUFFER;
> >     int err = inflateInit(&myZStream);
> >     if (err != Z_OK) {
> >       THROW (PersistException(String("zLib didn't initialise for 
> > inflating.")));
> >     }
> >   }
> 99,108c101,110
> <     {
> <       myZStream.next_in = myUncompressedDataBuffer;
> <       myZStream.next_out = myCompressedDataBuffer;
> <       myZStream.avail_in = 0;
> <       myZStream.avail_out = MAX_BUFFER;
> <       int err = deflateInit(&myZStream,9); // TODO: tweak compression level
> <       if (err != Z_OK) {
> <               THROW ( PersistException(String("zLib didn't initialise for 
> deflating.")));
> <       }
> <     }
> ---
> >   {
> >     myZStream.next_in = myUncompressedDataBuffer;
> >     myZStream.next_out = myCompressedDataBuffer;
> >     myZStream.avail_in = 0;
> >     myZStream.avail_out = MAX_BUFFER;
> >     int err = deflateInit(&myZStream,9); // TODO: tweak compression level
> >     if (err != Z_OK) {
> >       THROW ( PersistException(String("zLib didn't initialise for 
> > deflating.")));
> >     }
> >   }
> 117,119c119,121
> <     {
> <       inflateEnd(&myZStream);
> <     }
> ---
> >   {
> >     inflateEnd(&myZStream);
> >   }
> 121,134c123,136
> <     {
> <       int zret = Z_OK;
> <       while (myZStream.avail_in > 0 || zret == Z_OK)
> <             {
> <               zret = deflate(&myZStream,Z_FINISH);
> <               if (myZStream.avail_out >= 0)
> <                     {
> <                       
> myUnderlyingStream.write((char*)myCompressedDataBuffer,MAX_BUFFER - 
> myZStream.avail_out);
> <                       myZStream.next_out = myCompressedDataBuffer;
> <                       myZStream.avail_out = MAX_BUFFER;
> <                     }
> <             }
> <       deflateEnd(&myZStream);
> <     }
> ---
> >   {
> >     int zret = Z_OK;
> >     while (myZStream.avail_in > 0 || zret == Z_OK)
> >     {
> >       zret = deflate(&myZStream,Z_FINISH);
> >       if (myZStream.avail_out >= 0)
> >       {
> >         myUnderlyingStream.write((char*)myCompressedDataBuffer,MAX_BUFFER - 
> > myZStream.avail_out);
> >         myZStream.next_out = myCompressedDataBuffer;
> >         myZStream.avail_out = MAX_BUFFER;
> >       }
> >     }
> >     deflateEnd(&myZStream);
> >   }
> 148c150
> <     THROWS (Engine::Exception)
> ---
> >   THROWS (Engine::Exception)
> 150c152,153
> <   assert(myOperationalMode == modeWrite);
> ---
> >   if(myOperationalMode != modeWrite)
> >     THROW("Cannot write to an input Engine");
> 157,186c160,189
> <     {
> <       // transfer as much information as we can into the input buffer.
> <       if (myZStream.avail_in < MAX_BUFFER)
> <             {
> <               uint32 toAdd = size - written;
> <               if (toAdd > (MAX_BUFFER - myZStream.avail_in))
> <                     toAdd = (MAX_BUFFER - myZStream.avail_in);
> <               memcpy(myZStream.next_in + myZStream.avail_in, 
> data+written,toAdd);
> <               written += toAdd;
> <               myZStream.avail_in += toAdd;
> <             }
> <       if (myZStream.avail_in < MAX_BUFFER)
> <             return; // We have not filled the buffer, so let's carry on 
> streaming
> <       // We have a full input buffer, so we compressit.
> <       while (myZStream.avail_in > 0)
> <             {
> <               deflate(&myZStream,0);
> <               if (myZStream.avail_out == 0)
> <                     {
> <                       // We filled the output buffer, let's stream it
> <                       
> myUnderlyingStream.write((char*)myCompressedDataBuffer,MAX_BUFFER);
> <                       myZStream.next_out = myCompressedDataBuffer;
> <                       myZStream.avail_out = MAX_BUFFER;
> <                     }
> <               // Repeat whilst the input buffer isn't flushed
> <             }
> <       // Now we have flushed the input buffer we can reset it
> <       myZStream.avail_in = 0;
> <       myZStream.next_in = myUncompressedDataBuffer;
> <     }
> ---
> >   {
> >     // transfer as much information as we can into the input buffer.
> >     if (myZStream.avail_in < MAX_BUFFER)
> >     {
> >       uint32 toAdd = size - written;
> >       if (toAdd > (MAX_BUFFER - myZStream.avail_in))
> >       toAdd = (MAX_BUFFER - myZStream.avail_in);
> >       memcpy(myZStream.next_in + myZStream.avail_in, data+written,toAdd);
> >       written += toAdd;
> >       myZStream.avail_in += toAdd;
> >     }
> >     if (myZStream.avail_in < MAX_BUFFER)
> >     return; // We have not filled the buffer, so let's carry on streaming
> >     // We have a full input buffer, so we compressit.
> >     while (myZStream.avail_in > 0)
> >     {
> >       deflate(&myZStream,0);
> >       if (myZStream.avail_out == 0)
> >       {
> >         // We filled the output buffer, let's stream it
> >         myUnderlyingStream.write((char*)myCompressedDataBuffer,MAX_BUFFER);
> >         myZStream.next_out = myCompressedDataBuffer;
> >         myZStream.avail_out = MAX_BUFFER;
> >       }
> >       // Repeat whilst the input buffer isn't flushed
> >     }
> >     // Now we have flushed the input buffer we can reset it
> >     myZStream.avail_in = 0;
> >     myZStream.next_in = myUncompressedDataBuffer;
> >   }
> 192c195,196
> <   assert(myOperationalMode == modeRead);
> ---
> >   if(myOperationalMode != modeRead)
> >     THROW("Cannot read from an output Engine");
> 198,240c202,244
> <     {
> <       // If we have any data left in the uncompressed buffer - use it
> <       if (myLastUncompressedDataRead < myZStream.next_out)
> <             {
> <               uint32 toRead = size - read;
> <               if (toRead > (uint32)(myZStream.next_out - 
> myLastUncompressedDataRead))
> <                     toRead = (myZStream.next_out - 
> myLastUncompressedDataRead);
> <               memcpy(data+read,myLastUncompressedDataRead,toRead);
> <               myLastUncompressedDataRead += toRead;
> <               read += toRead;
> <             }
> <       if (read == size)
> <             return; // We have read all we need to
> <       // Reset the stream for the next block of data
> <       myLastUncompressedDataRead = myUncompressedDataBuffer;
> <       myZStream.next_out = myUncompressedDataBuffer;
> <       myZStream.avail_out = MAX_BUFFER;
> <       // Next we have to deal such that, until we have a full output buffer,
> <       // (Or we run out of input)
> <       if (myUnderlyingStream.good())
> <             {
> <               while (myUnderlyingStream.good() && myZStream.avail_out > 0)
> <                     {
> <                       // Right then, if we have run out of input, fetch 
> another chunk
> <                       if (myZStream.avail_in == 0)
> <                             {
> <                               myZStream.next_in = myCompressedDataBuffer;
> <                               
> myUnderlyingStream.read((char*)myCompressedDataBuffer,MAX_BUFFER);
> <                               myZStream.avail_in = 
> myUnderlyingStream.gcount();
> <                             }
> <                       inflate(&myZStream,0);
> <                     }
> <             }
> <       else
> <             {
> <               // Oh dear - we ran out of input on the buffer.
> <               // Maybe we can still inflate some
> <               inflate(&myZStream,0);
> <               if (myZStream.avail_out == MAX_BUFFER)
> < //                    THROW (PersistException(String("Oh dear - ran out of 
> input")));
> <                       THROW (Exception(String("Oh dear - ran out of 
> input")));
> <             }
> <     }
> ---
> >   {
> >     // If we have any data left in the uncompressed buffer - use it
> >     if (myLastUncompressedDataRead < myZStream.next_out)
> >     {
> >       uint32 toRead = size - read;
> >       if (toRead > (uint32)(myZStream.next_out - 
> > myLastUncompressedDataRead))
> >       toRead = (myZStream.next_out - myLastUncompressedDataRead);
> >       memcpy(data+read,myLastUncompressedDataRead,toRead);
> >       myLastUncompressedDataRead += toRead;
> >       read += toRead;
> >     }
> >     if (read == size)
> >     return; // We have read all we need to
> >     // Reset the stream for the next block of data
> >     myLastUncompressedDataRead = myUncompressedDataBuffer;
> >     myZStream.next_out = myUncompressedDataBuffer;
> >     myZStream.avail_out = MAX_BUFFER;
> >     // Next we have to deal such that, until we have a full output buffer,
> >     // (Or we run out of input)
> >     if (myUnderlyingStream.good())
> >     {
> >       while (myUnderlyingStream.good() && myZStream.avail_out > 0)
> >       {
> >         // Right then, if we have run out of input, fetch another chunk
> >         if (myZStream.avail_in == 0)
> >         {
> >           myZStream.next_in = myCompressedDataBuffer;
> >           myUnderlyingStream.read((char*)myCompressedDataBuffer,MAX_BUFFER);
> >           myZStream.avail_in = myUnderlyingStream.gcount();
> >         }
> >         inflate(&myZStream,0);
> >       }
> >     }
> >     else
> >     {
> >       // Oh dear - we ran out of input on the buffer.
> >       // Maybe we can still inflate some
> >       inflate(&myZStream,0);
> >       if (myZStream.avail_out == MAX_BUFFER)
> > //        THROW (PersistException(String("Oh dear - ran out of input")));
> >         THROW (Exception(String("Oh dear - ran out of input")));
> >     }
> >   }
> 250,251d253
> <   assert(myOperationalMode == modeWrite);
> < 
> 256,260c258,262
> <     {
> <       uint32 id = NullObject;
> <       write(id);
> <       return;
> <     }
> ---
> >   {
> >     uint32 id = NullObject;
> >     write(id);
> >     return;
> >   }
> 265,288c267,290
> <     {
> <       // Unfortunately we need to serialise it - here we go ....
> <       uint32 id = (uint32)myArchiveMap.size();
> <       myArchiveMap[object] = id; // bumps id automatically for next one
> <       write(id);
> <       ClassMap::const_iterator classItor = 
> myClassMap.find(object->getPersistenceID());
> <       if (classItor == myClassMap.end())
> <             {
> <               uint32 classId = (uint32)myClassMap.size();
> <               myClassMap[object->getPersistenceID()] = classId;
> <               write(classId);
> <               write(static_cast<String>(object->getPersistenceID()));
> <             }
> <       else
> <             {
> <               write(classItor->second);
> <             }
> <       String majik;
> <       majik = "OBST";
> <       write(majik);
> <       object->write(*this);
> <       majik = "OBEN";
> <       write(majik);
> <     }
> ---
> >   {
> >     // Unfortunately we need to serialise it - here we go ....
> >     uint32 id = (uint32)myArchiveMap.size();
> >     myArchiveMap[object] = id; // bumps id automatically for next one
> >     write(id);
> >     ClassMap::const_iterator classItor = 
> > myClassMap.find(object->getPersistenceID());
> >     if (classItor == myClassMap.end())
> >     {
> >       uint32 classId = (uint32)myClassMap.size();
> >       myClassMap[object->getPersistenceID()] = classId;
> >       write(classId);
> >       write(static_cast<String>(object->getPersistenceID()));
> >     }
> >     else
> >     {
> >       write(classItor->second);
> >     }
> >     String majik;
> >     majik = "OBST";
> >     write(majik);
> >     object->write(*this);
> >     majik = "OBEN";
> >     write(majik);
> >   }
> 290,293c292,295
> <     {
> <       // This object has been serialised, so just pop its ID out
> <       write(itor->second);
> <     }
> ---
> >   {
> >     // This object has been serialised, so just pop its ID out
> >     write(itor->second);
> >   }
> 301d302
> <   assert(myOperationalMode == modeRead);
> 304c305,306
> <   assert(id != NullObject); // id should never be null for a reference
> ---
> >   if (id == NullObject)
> >     THROW("Object Id should not be NULL when unpersisting to a reference");
> 308,311c310,313
> <     {
> <       object = *(myArchiveVector[id]);
> <       return;
> <     }
> ---
> >   {
> >     object = *(myArchiveVector[id]);
> >     return;
> >   }
> 326d327
> <   assert(myOperationalMode == modeRead);
> 331,334c332,335
> <     {
> <       object = NULL;
> <       return;
> <     }
> ---
> >   {
> >     object = NULL;
> >     return;
> >   }
> 338,341c339,342
> <     {
> <       object = myArchiveVector[id];
> <       return;
> <     }
> ---
> >   {
> >     object = myArchiveVector[id];
> >     return;
> >   }
> 356,357c357,358
> <     {
> <       // Okay then - we can make this object
> ---
> >   {
> >     // Okay then - we can make this object
> 359c360
> <     }
> ---
> >   }
> 361c362
> <       THROW (Exception((String("Unable to instantiate object of class 
> ")+className).c_str()));
> ---
> >     THROW (Exception(String("Unable to instantiate object of class 
> > ")+className));
> 369,376c370,379
> <     // Okay then - we can make this object
> <     myArchiveVector.push_back(object);
> <     String majik;
> <     read(majik);
> <     assert(majik == String("OBST"));
> <     object->read(*this);
> <     read(majik);
> <     assert(majik == String("OBEN"));
> ---
> >   // Okay then - we can make this object
> >   myArchiveVector.push_back(object);
> >   String majik;
> >   read(majik);
> >   if(majik != String("OBST"))
> >     THROW("Missing Start-of-Object marker");
> >   object->read(*this);
> >   read(majik);
> >   if(majik != String("OBEN"))
> >     THROW("Missing End-of-Object marker");
> 389,391c392,394
> <     {
> <       className = myClassVector[classId];
> <     }
> ---
> >   {
> >     className = myClassVector[classId];
> >   }
> 393,397c396,400
> <     {
> <       // Okay the class wasn't known yet - save its name
> <       read(className);
> <       myClassVector.push_back(className);
> <     }
> ---
> >   {
> >     // Okay the class wasn't known yet - save its name
> >     read(className);
> >     myClassVector.push_back(className);
> >   }
> 409d411
> <   assert(myOperationalMode == modeWrite);
> 417d418
> <   assert(myOperationalMode == modeRead);
> 433d433
> <   assert(myOperationalMode == modeWrite);
> 441d440
> <   assert(myOperationalMode == modeRead);
> 477c476
> < #ifdef      HAVE_64_BITS
> ---
> > #ifdef  HAVE_64_BITS
> 498c497
> <     uint32 a; ar.read(a); ob=a==1;return ar;
> ---
> >   uint32 a; ar.read(a); ob=a==1;return ar;
> 501c500
> <     uint32 a=ob?1:0; ar.write(a); return ar;
> ---
> >   uint32 a=ob?1:0; ar.write(a); return ar;
> 507c506
> < #ifdef      CCXX_NAMESPACES
> ---
> > #ifdef  CCXX_NAMESPACES

> _______________________________________________
> Bug-commoncpp mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/bug-commoncpp