Re: [boost] [AFIO] Review (or lack of it)

Subject: Re: [boost] [AFIO] Review (or lack of it)
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2015-08-29 12:14:28

On 29 Aug 2015 at 18:58, Andrey Semashev wrote:

> > I always code in detection of impossible states and fatal exit in any
> > of my code and I have done so for years. It has proven its worth time
> > and time again. I obviously don't go overboard with O(N^2) proof of
> > good state checks in release builds, but if detection of impossible
> > states has a low overhead, it's worth doing as a matter of routine.
> > 
> > Think of it being a manually written undefined behaviour sanitiser.
> > It's no different.
> 
> If you're trying to protect yourself from a memory failure this way then I 
> think your approach is futile. You can't guarantee correctness with your 
> checks, however many you insert in the code. If you want better guarantees buy 
> a better hardware.
> 
> If you're trying to protect from a program bug that results in memory 
> corruption (e.g. through a dangling pointer) then terminating with 
> std::terminate is useless because it doesn't help to locate and eliminate the 
> bug. It doesn't help the customer either since his production service crashes 
> all the time. You may save the data from corruption but again - there are no 
> guarantees as the corrupted data may have slipped through. I guess this is 
> better than nothing but hardly something I could count on.

Remember that AFIO is reading and writing other people's data. If 
internal state has become corrupt, AFIO no longer knows if the 5Mb of 
data to be written at offset X in file Y is right or wrong. If it 
proceeded, you risk corruption and damage to other people's data.

This is why I make no apologies for immediate fatal exit of the 
process in this situation.

If AFIO were not reading and writing other people's data, I'd be far 
happier to do something less extreme.

> > To be honest it's never come up. In use cases without memory
> > corruption you never see this code in action. On the other hand, when
> > someone logs a bug which mentions all this stuff printed to
> > std::cerr, it's an excellent sign they have memory corruption
> > problems.
> 
> So the checks never fired so far but you say they have proven their worth many 
> times. In what way?

Oh they do fire for me during my development, but very rarely for 
commits pushed to master branch. By that stage they are hopefully 
debugged.

> > > I think you're trying to be too smart in your library. Don't. You will
> > > never cover everything users want. There will always be someone like me
> > > who wants a different behavior than you envisioned.
> > 
> > I am not being too smart in my library. I am choosing sensible
> > defaults which are useful in most use cases to most people. All this
> > is really quality of implementation detail anyway.
> 
> Trying to protect from memory failures is one example of being too smart. This 
> isn't the library's job to do. Console output also doesn't look like something 
> universally accepted.

AFIO doesn't go to extremes to check for memory corruption. It does 
some basic checks for internal consistency, ones which are 
lightweight and cost very little.

The console output is already logged with an issue to change that to 
a macro so it can be changed by anyone requiring that.

> > Issues have been logged for controlling the printing to std::cerr and
> > enabling or disabling the deadlock debug printing code. If at a later
> > stage people find they need additional behaviours in their real world
> > use cases I have no problem adding solutions for them as well.
> 
> In general I'd discourage from adding configuration macros as they increase 
> the possibility of config mismatch between TUs. Please consider macro-free 
> options first.

These are internal code on vs off switches. They have no effect on 
ABI, and if an end user wants a local customised variant which 
coexists with other copies of AFIO it's very easy to make one thanks 
to APIBind.

Niall

-- 
ned Productions Limited Consulting
http://www.nedproductions.biz/ 
http://ie.linkedin.com/in/nialldouglas/