[Spice-devel] [PATCH 12/22] Add exception handling classes

[Christophe Fergeau]

On Fri, Mar 02, 2018 at 06:06:32PM +0100, Christophe de Dinechin wrote:
> > FWIW, I also find myself in agreement with Frediano and Lukas on this point.
> 
> OK. Maybe I misunderstood something.
> 
> Do we agree that the case Frediano raised is someone trying:
> 
> 	throw Error(“The value of “ + std::to_string(2) + “ does not match “ + std::to_string(1));
> 
> which yields the following error:
> 
> 	spice-streaming-agent.cpp:165:93: error: no matching function for call to ‘spice::streaming_agent::Error::Error(std::__cxx11::basic_string<char>)’
> 
> So far, I think the interface was not “error prone”. It caught the error correctly.
> 
> Therefore, in order to find the interface “error prone”, one has to accept a second hypothesis, which is that whoever attempted that, after seeing that error message, looks at the interface, which has a ‘const char *’ and a comment that specifically states "It is recommended to only use static C strings, since formatting of any dynamic argument is done by ‘format_message’”, and after reading that comment, has a big “Aha” moment and writes:
> 
> 	throw Error((“The value of “ + std::to_string(2) + “ does not match “ + std::to_string(1)).c_str());
> 
> I’m sorry, but I cannot accept that step of the reasoning as being even remotely valid. Just looking at the code makes me want to barf.
> 
> So either I misunderstood Frediano’s point, or the reasoning is deeply flawed in a not very subtle way.

It's not just about .c_str(), it's really about any API returning a
non-static string which will be freed before the exception is caught
(think RAII-like cases, wrappers around preexisting C APIs, ...).
I already mentioned it, but an API taking a const char *, and also
mandating that this const char * must be alive as long as the instance
I invoked the method from is really unusual/unexpected to me, and would
thus be error-prone.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180306/d8e4269c/attachment.sig>