> On 6 Mar 2018, at 17:47, Jonathon Jongsma <jjongsma@xxxxxxxxxx> wrote:
>
> 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:
Your example is basically a variant of mine.
> ------
>
> #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
Yes. Why did you add c.str() to start with?
If I write this:
typedef std::vector<const char *> ArgumentList;
ArgumentList build_args_list(int argc, char **argv)
{
ArgumentList argList;
for (int a = 1; a < argc; a++)
{
std::string arg = argv[a];
// … do some other stuff
argList.push_back(arg.c_str()); // dangerous
}
return argList;
}
do I have a right to complain to the writers of the standard library that their interface is “error prone"?
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel