Serial Port OK in class constructor but not outside...

Mike Grove

I am opening a COM port in a class constructor - no problem. I can send some data at this point too - no problem. BUT, if I try to send data from any other method in the class it appears to work ok (I get the expected ERROR_IO_PENDING), but subsequently my code suffers an unpredictable and undebuggable crash. Any ideas anyone (please ask for more info' if needed)... Some code;

CSerialDevice::CSerialDevice(
CString& strDeviceId,
CString& strDeviceName,
CString& COMPort,
CString& BaudRate,
CString& Format,
ComWrapper_c Db, GobList_t& rGobList,
CSerialIOInterface* pInterface )
: pOurInterface( pInterface ),
m_hCommPort( NULL )
{
// Create the items for the device
AddDigitalItems( strDeviceName, strDeviceId, Db, rGobList );
AddAnalogueItems( strDeviceName, strDeviceId, Db, rGobList );

// Configure the COM port for the device
if ( !ConfigureSerialPort( COMPort, BaudRate, Format ) )
{
	pOurInterface->AddErrorToStatus( "Failed to configure port for " + strDeviceName );
}

}

/////////////////////////////////////////////////////////////////////////////
//
//
//
/////////////////////////////////////////////////////////////////////////////
bool CSerialDevice::ConfigureSerialPort(
CString& strCOMPort,
CString& strBaudRate,
CString& strFormat )
{
// Open the port
m_hCommPort = CreateFile(
strCOMPort,
GENERIC_READ | GENERIC_WRITE,
0,
0,
OPEN_EXISTING,
FILE_FLAG_OVERLAPPED,
0 );

// Get the default configuration...
DCB dcb = {0};
dcb.DCBlength = sizeof(DCB);
if ( !GetCommState( m\_hCommPort, &dcb ) )
{
	return false;
}

// ...and change the items we want to
dcb.BaudRate = atoi( strBaudRate );
dcb.ByteSize = atoi( strFormat.Left( 1 ) );
int parity = NOPARITY;
if ( strFormat.Mid( 1 ) == 'N' )
{
	parity = NOPARITY;
}
else if ( strFormat.Mid( 1 ) == 'E' )
{
	parity = EVENPARITY;
}
else if ( strFormat.Mid( 1 ) == 'O' )
{
	parity = ODDPARITY;
}
dcb.Parity = parity;
int stopBits = atoi( strFormat.Right( 1 ) );
switch ( stopBits )
{
case 0  : stopBits = ONESTOPBIT; break;
case 2  : stopBits = TWOSTOPBITS; break;
default : stopBits = ONESTOPBIT; break;
}
dcb.StopBits = stopBits;
if ( !SetCommState( m\_hCommPort, &dcb ) )
{
	return false;
}

COMMTIMEOUTS timeouts;
timeouts.ReadIntervalTime

Jochen Arndt

CSerialDevice::TransmitData( int bytesToSend )
{
OVERLAPPED ovlWrite = {0};
WriteFile( m_hCommPort, mTxBuf, bytesToSend, NULL, &ovlWrite );
}

The OVERLAPPED structure ovlWrite is a local variable that goes out of scope when leaving the function. But when writing is finished later, the system tries to access it. To avoid this, make ovlWrite a member variable of your class or make it static.

etkid84

I try to keep code that "does stuff" out of the constructor and limit that method to doing attribute assignments. If I am correct, if something barfs in the constructor you have no way of handling the mess gracefully and recovering. Isn't there some kind of create or init method that you can use?

David

pasztorpisti

The guys have already given you good hints, I give you another one: Never call virtual methods from the constructor and destructor of your classes because at the time the constructor and destructor is executed the vtable is set the the vtable of the class whose ctor/dtor is being executed even if this object is the instance of a derived class that would have override for the called virtual methods. This leads to a lot of errors and this C++ glitch also implies that serious initialization in the ctor is discouraged.

Richard Andrew x64

pasztorpisti wrote:

the vtable is set the the vtable of the class whose ctor/dtor is being executed even if this object is the instance of a derived class

If you're constructing the object, the compiler knows the correct type at the time of construction, so how is the vtable going to be wrong?

The difficult we do right away... ...the impossible takes slightly longer.

Mike Grove

Many thanks, Jochen, that was the solution! (I live and learn...).

Mike Grove

Thank-you, David. That wasn't the solution (see Jochen's reply above), but yours was still good advice...

pasztorpisti

EDIT: previously I stated this has something to do with the closed world asssumptions of the compiler but I'm not sure about that. This thing works this way: It has no point to call the virtual method of a derived class whose constructor hasn't yet been executed because that virtual method would have access to uninitiailized members. In java and C# such virtual call calls the derived virtual method in contrast to C++!!!! But there at least the derived class members are already zero initialized, still, that is also the source of evil bugs! I had the luck to fix one such bug in java. Then why doesn't the compiler tell me about direct/indirect virtual calls from my ctor/dtor??? Because it can't always do that and even in some cases where it could tell it doesn't do that, probably the compiler writers didn't want to waste time on that or maybe this is the intended behavior even if its useless and often the source of bugs. The code of the constructor of each c++ class looks like the following (Derived1 inherits from Base, Derived2 inherits from Derived1:

Base::Base()
{
auto generated: vtable = Base::vtable
}

Derived1::Derived1()
{
auto generated: call Base::Base
auto generated: vtable = Derived1::vtable
}

Derived2::Derived2()
{
auto generated: call Derived1::Derived1
auto generated: vtable = Derived2::vtable
}

The destructors:

Base::~Base()
{
auto generated: vtable = Base::vtable
}

Derived1::~Derived1()
{
auto generated: vtable = Derived1::vtable
auto generated: call Base::~Base
}

Derived2::~Derived2()
{
auto generated: vtable = Derived2::vtable
auto generated: call Derived1::~Derived1
}

With the above info guess what will be printed by this example program:

class Base
{
public:
Base()
{
VirtualCall();
}
~Base()
{
VirtualCall();
}

virtual void VirtualCall()
{
    printf("Base::VirtualCall()\\n");
}

};

class Derived : public Base
{
public:
virtual void VirtualCall()
{
printf("Derived::VirtualCall()\n");
}
};

void TestFunc()
{
Derived* d = new Derived;
delete d;
}

You should never do complex initialization and deinitialization in your ctor/dtor because the virtual call may be hidden i

Richard Andrew x64

OK, now it makes sense. Thanks, I learned something valuable.

The difficult we do right away... ...the impossible takes slightly longer.

pasztorpisti

I forgot to mention something: If your class in your class hierarchy is abstract or you simply know that you never create an instance of that class then you can use the __declspec(novtable) on the class in VC++. This way the ctor won't contain the automatically generated vtable initializer. If you don't call any virtual methods from the ctor (and you shouldn't) then its enough to have vtable initialization only in the actual class ctor from which you create the instance (in the "most derived" one). In practice I used this optimization only once because usually this thing is not the bottleneck.