Description:
Several memory allocations using new in TransporterRegistry and Transporter are not checked for memory allocation error.
TransporterRegistry.cpp:
theTCPTransporters = new TCP_Transporter * [maxTransporters];
theSCITransporters = new SCI_Transporter * [maxTransporters];
theSHMTransporters = new SHM_Transporter * [maxTransporters];
theTransporterTypes = new TransporterType [maxTransporters];
theTransporters = new Transporter * [maxTransporters];
performStates = new PerformState [maxTransporters];
ioStates = new IOState [maxTransporters];
m_disconnect_errnum = new int [maxTransporters];
m_error_states = new ErrorState [maxTransporters];
Transporter.cpp:
m_socket_client= new SocketClient(remoteHostName, s_port,
new SocketAuthSimple("ndbd",
"ndbd passwd"));
m_socket_client->set_connect_timeout((m_timeOutMillis+999)/1000);
TransporterRegistry.cpp example of bad error handling:
m_send_buffer_memory =
new unsigned char[UintPtr(send_buffer_pages * SendBufferPage::PGSIZE)];
if (m_send_buffer_memory == NULL)
{
ndbout << "Unable to allocate "
<< send_buffer_pages * SendBufferPage::PGSIZE
<< " bytes of memory for send buffers, aborting." << endl;
abort();
}
How to repeat:
MCI
Suggested fix:
The TransporterRegistry or Transporter constructor can't return an error code so the check(and maybe also the allocation) should be deffered to a 'bool init()' function, which should return false if memory can't be allocated or other error occurs during creation/construction of the Transporter* objects.
If an error occurs, an error message describing the problem should be printed to stderr and the function return false.'
There is also one place in TransporterRegistry where the allocation _is_ checked and error is handled by aborting, it should be modified to also return false and thus giving the caller the option to decide how to handle the error.
Description: Several memory allocations using new in TransporterRegistry and Transporter are not checked for memory allocation error. TransporterRegistry.cpp: theTCPTransporters = new TCP_Transporter * [maxTransporters]; theSCITransporters = new SCI_Transporter * [maxTransporters]; theSHMTransporters = new SHM_Transporter * [maxTransporters]; theTransporterTypes = new TransporterType [maxTransporters]; theTransporters = new Transporter * [maxTransporters]; performStates = new PerformState [maxTransporters]; ioStates = new IOState [maxTransporters]; m_disconnect_errnum = new int [maxTransporters]; m_error_states = new ErrorState [maxTransporters]; Transporter.cpp: m_socket_client= new SocketClient(remoteHostName, s_port, new SocketAuthSimple("ndbd", "ndbd passwd")); m_socket_client->set_connect_timeout((m_timeOutMillis+999)/1000); TransporterRegistry.cpp example of bad error handling: m_send_buffer_memory = new unsigned char[UintPtr(send_buffer_pages * SendBufferPage::PGSIZE)]; if (m_send_buffer_memory == NULL) { ndbout << "Unable to allocate " << send_buffer_pages * SendBufferPage::PGSIZE << " bytes of memory for send buffers, aborting." << endl; abort(); } How to repeat: MCI Suggested fix: The TransporterRegistry or Transporter constructor can't return an error code so the check(and maybe also the allocation) should be deffered to a 'bool init()' function, which should return false if memory can't be allocated or other error occurs during creation/construction of the Transporter* objects. If an error occurs, an error message describing the problem should be printed to stderr and the function return false.' There is also one place in TransporterRegistry where the allocation _is_ checked and error is handled by aborting, it should be modified to also return false and thus giving the caller the option to decide how to handle the error.