Re: [lmi] Danger of fatal_error()

[Greg Chicares]

On 2016-02-25 00:40, Vadim Zeitlin wrote:
> On Fri, 12 Feb 2016 23:12:36 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> Ultimately I anticipate integrating this into lmi, where I'd prefer to use
> GC> lmi's fatal_error() instead...
> GC> 
> GC>     fatal_error()
> GC>         << "Value for numeric field '"
> GC>         << name
> GC>         << "' is out of range (maximum allowed is "
> GC>         << max_num
> GC>         << ")."
> GC>         << LMI_FLUSH // A macro, yes, but a minimal, unavoidable one.
> GC>         ;
> 
>  I'd just like to know that this macro (or at least non-macro std::flush)
> may well be minimal and unavoidable, but it's still quite dangerous because
> the code compiles without any problems if you forget it but, of course,
> doesn't work at all during run-time.
> 
>  In my case I had something like
> 
>       void do_something(some_type* ptr)
>       {
>           if(!ptr)
>           {
>           fatal_error() << ... long message ...;
>           }
> 
>           ... dereference and use ptr ...
>       }
> 
> and it took me quite some time to understand how this code could crash
> dereferencing this pointer.

I wanted to allow this:

    std::vector<std::string> v;
    ...
    fatal_error() << " list: ";
    std::copy
        (v.begin()
        ,v.end()
        ,std::ostream_iterator<std::string>(fatal_error(), " ")
        );
    fatal_error() << LMI_FLUSH;

It's a tradeoff, documented here:

/// The output destination could easily be expressed as a manipulator:
///   single_ostream << "error" << some_variable << messagebox;
/// That might be a slightly simpler design. And it is intended that an
/// exception be thrown for fatal errors at least, which seems more
/// like an independent action than a consequence of flushing a stream.
/// But following the std::cerr paradigm is the least surprising
/// approach, and it seems natural enough to emit the contents of the
/// buffer when std::flush is called.

>  Maybe I'm just particularly prone to errors, but I think it would be still
> nice to use C++11 variadic function to write the above as
> 
>         fatal_error
>             ("Value for numeric field '"
>             ,name
>             ,"' is out of range (maximum allowed is "
>             ,max_num
>             ,")."
>             );

That's a different design. As usual, I'm averse to changing the design
of a widely-used low-level facility without a compelling reason. If
there's a natural and non-intrusive way to signal an error when leaving
a scope with an unflushed stream:

  if(condition)
    {
    fatal_error() << "Not flushed...";
    }

then we could talk about that; but I don't think it can work because
  std::ostream& LMI_SO fatal_error();
it's a reference, not an object on the stack.