[Gnash-dev] Should gnash::rtmp::RTMP::connect throw exceptions?

[Petter Reinholdtsen]

Hi.  While looking at the Coverity report on unhandled exceptions, I
ran into a few questions.  A handful ignored exceptions is in the
utilities/rtmpget.cpp file, and this email is about one particular
set.  The file contain this part:

    gnash::rtmp::RTMP r;
    [...]
    log_debug("Initial connection");

    if (!r.connect(url)) {
        log_error("Initial connection failed!");
        std::exit(EXIT_FAILURE);
    }

Coverity claim r.connect(url) can throw two exceptions,
gnash::GnashException and
boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::bad_lexical_cast>>,
and I believe it.  But should it?

The documentation for the connect() function state that it should
return false on problems, not throw:

    /// Initiate a network connection.
    //
    /// Note that this only creates the TCP connection and carries out the
    /// handshake. An active data connection needs an AMF connect request,
    /// which is not part of the RTMP protocol.
    //
    /// @return     true if the connection attempt starts, otherwise false.
    ///             A return of false means that the RTMP object is in a 
    ///             closed state and can be reconnected.
    bool connect(const URL& url);

Would the correct fix be to add 'throw ()' for that function to
document that it should never throw exceptions, and handle the
exceptions internally in connect().  Or is it to add code to catch the
exceptions in rtmpget.cpp and the other places that might get the
exceptions?  If so, I guess the documentation should be extended to
mention that connect() can throw?

-- 
Happy hacking
Petter Reinholdtsen