Re: [PATCH 12/22] Add exception handling classes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 02, 2018 at 02:00:23PM +0100, Christophe de Dinechin wrote:
> 
> 
> > On 2 Mar 2018, at 09:03, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> > 
> >>> 
> >>> On 1 Mar 2018, at 13:13, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> >>> 
> >>>> 
> >>>> From: Christophe de Dinechin <dinechin@xxxxxxxxxx>
> >>>> 
> >>>> Throwing 'runtime_error' directly should be reserved for the support
> >>>> library.  Add an 'Error' class as a base class for all errors thrown
> >>>> by the streaming agent, as well as subclasses used to discriminate
> >>>> between categories of error.
> >>>> 
> >>>> The Error class offers:
> >>>> - a 'format_message' member function that formats a message without
> >>>> allocating memory, so that there is no risk of throwing another
> >>>> exception at throw time.
> >>>> 
> >>> 
> >>> why not formatting the message constructing the object so before the throw?
> >> 
> >> That requires dynamic allocation, which is entirely unnecessary and
> >> potentially dangerous (replacing one exception kind with another).
> >> 
> > 
> > As explained by my string.c_str example your interface is prone to errors.
> 
> The interface is not “prone to errors” at all. The scenario you
> describe makes no sense, since the whole interface is designed to
> avoid dynamic strings. This is now explicitlyi documented (see
> https://github.com/c3d/spice-streaming-agent/blob/c%2B%2B-refactoring/include/spice-streaming-agent/errors.hpp,
> not yet shared, I’m not entirely done with the reviews). The
> constructor comment now says:
> 
> 	Constructor takes the base message returned by what()
> 	The message must remain valid over the whole lifetime of the exception.
> 	It is recommended to only use static C strings, since formatting of any
> 	dynamic argument is done by 'format_message' 

This behaviour seems inconsistent with std::runtime_error(const char *)
which as far as I can tell does not have this lifetime expectation.

> 
> Anyway, your example is misleading. Anybody that passes the result of
> string::c_str() to a const char * without proper consideration for the
> lifetime is at fault. The const char * interface itself cannot be
> faulted for that.

For what it's worth, I usually expect const char * interfaces to make
their own copy of the const char * arg if they need it after they
return, and I've rarely seen exceptions to this expectation.

If I understood properly, things are designed this way to avoid running
out of memory while building the exception. I'm definitely not in favour
of having an error-prone API to handle a very rare case which the rest
of the code is most likely not going to be able to cope with anyway.

Christophe

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]