Bug #53936 Memory allocations not checked in TransporterRegistry and Transporter's
Submitted: 24 May 2010 9:24
Reporter: Magnus Blåudd Email Updates:
Status: Verified Impact on me:
Category:MySQL Cluster: Cluster (NDB) storage engine Severity:S2 (Serious)
Version:mysql-5.1-telco-7.0 OS:Any
Assigned to: Magnus Blåudd CPU Architecture:Any
Tags: 7.0.15

[24 May 2010 9:24] Magnus Blåudd
Several memory allocations using new in TransporterRegistry and Transporter are not checked for memory allocation error.

  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];

    m_socket_client= new SocketClient(remoteHostName, s_port,
				      new SocketAuthSimple("ndbd",
							   "ndbd passwd"));


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;

How to repeat:

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.