[Quickfix-developers] read errors in readFromSocket losing information in Exception Handling

[Howard E. <how...@pi...>]

If the recv  call in SocketConnection::readFromSocket receives <=3D 0
bytes on a recv call it throws a SocketRecvFailed exception, passing the
return value to the constructor.  The likely return values will be 0
(EOF on Socket) or (-1) Error (evaluate errno for specific error
condition).

void SocketConnection::readFromSocket()
throw( SocketRecvFailed )
{
  int size =3D recv( m_socket, m_buffer, 4095, 0 );
  if( size <=3D 0 )
    throw SocketRecvFailed( size );
  m_buffer[ size ] =3D '\0';

  m_parser.addToStream( m_buffer, size );
}

The SocketRecvFailed struct derives from SocketException.  If its
constructor receives no value it will format a message based on errno.
Otherwise it captures the string message passed in the overloaded
constructor.

/// Socket Error
struct SocketException : public Exception
{
  SocketException()
    : Exception( "Socket Error", errorToWhat() ) {}

  SocketException( const std::string& what )
    : Exception( "Socket Error", what ) {}

  std::string errorToWhat()
  {
#ifdef _MSC_VER
    error =3D WSAGetLastError();
    char buffer[2048];
    FormatMessage( FORMAT_MESSAGE_FROM_SYSTEM, NULL, error,
                   MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
                   buffer, 2048, NULL );
    return buffer;
#else
    error =3D errno;
    return strerror( error );
#endif
  }

  int error;
};

So the SocketRecvFailed constructor is passed the recv return value (0
or -1).  As we see below, if it is passed 0 it calls the base
implementation with "Connection reset by peer." message.  I would argue
that this is not the best choice of messages as it technically pertains
to ECONNRESET.  "EOF on socket" may be more accurate.  If it is passed <
0 (error) it passes the base constructor "".  I believe the intent was
to have the SocketException() base constructor invoked, calling
errorToWhat to get the error string from errno.  However the actual
result is that the SocketException is passed a blank string and no errno
specific information is captured.

/// Socket recv operation failed
struct SocketRecvFailed : public SocketException
{
  SocketRecvFailed( int size )
    : SocketException( size =3D=3D 0 ? "Connection reset by peer." : =
size <
0 ? "" : "Success." ) {}
  SocketRecvFailed( const std::string& what )
    : SocketException( what ) {}
};