On Tue, 2018-03-06 at 11:02 +0100, Christophe Fergeau wrote:
> 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<ch
> > ar>)’
> >
> > 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
A simple concrete example:
------
#include <exception>
#include <iostream>
#include <string>
class Error final : public std::exception
{
public:
Error(const char *message): exception(), message(message) { }
virtual const char *what() const noexcept override { return
message; }
protected:
const char *message;
};
void do_it(int x)
{
std::string foo("Hello, world");
// do some other stuff...
if (x == 1)
throw Error(foo.c_str()); // dangerous
else if (x == 2)
throw std::runtime_error(foo.c_str()); // fine
std::cout << "run without any arguments for Error, or with 1
argument for runtime_error";
}
int main(int argc, char** argv)
{
try {
do_it(argc);
} catch (Error& ex) {
std::cout << "Error: " << ex.what() << std::endl;
} catch (std::runtime_error& ex) {
std::cout << "runtime_error: " << ex.what() << std::endl;
}
return 0;
}
------
The fact that the interface is exactly the same as runtime_error but it
behaves differently is almost the textbook definition of error-prone,
IMO.
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
