Hi Atul,
I agree it is hard to achieve strong exception safety, especially in the standard template library where little assumptions can be made about the the template types. We have the advantage that we are both the user and the client of our code and most of the time we know the concrete types of the objects we are using. I would suggest that we try to achieve strong exception safety and see where we have difficulties. From what I read, the standard library tries to achieve the ’strong’ level where the ‘no throw’ level is not possible and the ‘basic’ level when the ’strong’ is not possible / efficient enough. I guess we could use a similar approach.
As a first try I wanted to make StreamBuffer exception-safe. I found the dtor doesn’t free dynamically allocated memory (auto_ptr is not appropriate as we are allocating char arrays). The only two functions I found could throw an exception are setWriteBufferSize() and setReadBufferSize() and that would happen if new[]() throws. All that is required to make the functions safe is to reorder the operations, namely the memory allocation should be performed before modifying the object state:
=== modified file 'Protocols/Generics/StreamBuffer.cpp'
--- Protocols/Generics/StreamBuffer.cpp 2007-03-15 23:14:13 +0000
+++ Protocols/Generics/StreamBuffer.cpp 2007-04-16 19:47:54 +0000
@@ -35,6 +35,8 @@
//! Destructs a StreamBuffer object.
StreamBuffer::~StreamBuffer()
{
+ delete[] readBuffer;
+ delete[] writeBuffer;
}
//! Tells whether \a count bytes can be read from the read buffer.
@@ -104,8 +106,9 @@
void StreamBuffer::setReadBufferSize (int size)
{
Q_ASSERT (size >= 0);
+ char *newBuffer = new char [size];
delete[] readBuffer;
- readBuffer = new char [size];
+ readBuffer = newBuffer;
readSize = size;
readStart = readEnd = 0;
}
@@ -215,8 +218,9 @@
void StreamBuffer::setWriteBufferSize (int size)
{
Q_ASSERT (size >= 0);
+ char *newBuffer = new char [size];
delete[] writeBuffer;
- writeBuffer = new char [size];
+ writeBuffer = newBuffer;
writeSize = size;
writeStart = writeEnd = 0;
}
In the old code, if an exception was thrown and the destructor of StreamBuffer was called when unwinding the stack, the buffers would have been deleted twice (assuming the dtor leak was fixed).
Yes, this class is too simple but we got improvement at a very little price. We should definitely try that for other classes as well.
Could you identify further problems with StreamBuffer?
Regards,
Peter
