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

[Sandro Santilli]

On Thu, Apr 24, 2014 at 11:16:19AM +0200, Petter Reinholdtsen wrote:
> 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?

I think it'd better not throw the boost one.

Exceptions are more flexible in that they can report different kind
of errors while the boolean return can only express success/failure.
Do we need detail ?
In which case is GnashException thrown ?

> 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?

Exception specification in the signature is a discouraged habit:
http://programmers.stackexchange.com/questions/114338/why-are-exception-specifications-bad
Documentation is always good.

--strk;