Subject: Re: [boost] [AFIO] Review (or lack of it)
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2015-08-28 05:31:03
On 28.08.2015 05:28, Niall Douglas wrote:
> On 27 Aug 2015 at 12:54, Roland Bock wrote:
>> * "The /final/ named blob store design" starts with "Caution: This
>> section was not finished".
>> * "The performance of and problems with this third generation design"
>> consists of "This section was not completed in time[...]"
>> * So basically the crucial parts of the blob store sequence are
>> incomplete at best.
> The final example is over 1000 lines of code. I didn't think it
> mattered hugely to the review.
You mentioned this earlier as well. I keep wondering why an example
requires more than 1000 lines of code? Is it because the task is too
broad or because the library interface is too verbose? In both cases
something should be done about it - either change the docs or the library.
>> f) Code (since the implementation is going to change anyway, this might
>> all be useless):
>> * /attic: Seriously?
> AFIO will be three years old soon. It had a life before being ported
> to Boost in 2013. There are still bits and pieces in the attic
> directory of use to me.
The implementation in Boost should preferably be free from lecacy code.
If that code is generally useful then why is it not in the library? If
it's only usefult to you then keep it in your own repo separate from the
>> * I am not a big fan of #if 0 in release code
> For the examples, logged at
> In the implementation, if it's #if 0 it's almost always debug assist
> code which I turn on from time to time to help fix some reported bug.
> Or it's code which isn't fully baked yet and end users shouldn't use
> it e.g. byte range locking.
>> * I doubt that std::cerr should be in this library's code.
> Use of std::cerr is only when something truly unusual occurs, like we
> are about to fatal exit the process and I would assume the user would
> like to know why we just called std::terminate.
Please, don't do that (both print anything and call std::terminate). In
my projects I'm very often required to remove any output to the console
and redirect it to a (custom) log library. Third party libraries that
have hard-coded console output are always a pain and have to be patched.
Calling std::terminate is also something I would call unexpected and
harmful. You don't know what the application is doing - one thread may
be doing something with Boost.AFIO and another may be doing domething
important and unrelated. Throw exceptions on errors, where possible.
Document UB when not.
> Or we are about to
> deadlock, and again the user probably wants to know why their
> application has just hanged itself.
Let it deadlock. Seriously. At least the user will be able to see it
blocked and get a backtrace to figure out how did it come to this.
Terminating the app is the most useless and harmful reaction.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk