Subject: [boost] [stacktrace] Review
From: Bjorn Reese (breese_at_[hidden])
Date: 2017-03-25 12:54:21
I vote for the unconditional acceptance of Boost.Stacktrace.
The design is sound. The separation of stacktrace and frame is just
right, and safe_dump_to is an elegant solution to handle async-safety
(much better than the fork-exec alternative.)
The implementation (I did not look at the Windows parts) looks solid.
There are details that looks platform-specific -- e.g. "/proc/self/exe"
in detail/addr2line_impls.hpp is not Posix -- but I did not notice
anything that cannot be ported if needed.
The documentation is well-structured. I appreciate that the reference
documentation contains both complexity and async-handler-safety
This library is very useful to developers.
I spent a couple of hours going through the documentation and source
for the review. Prior to that I have followed the development of
this library, and used an earlier version in a commercial product.
I have in-depth knowledge of the Posix side (having once implemented
an async-safe stacktrace.)
Make throw_with_trace of the API. Put it in a separate header so
there is no dependency on Boost.Exception unless this header is
included. In order to be consistent with Boost.Exception I suggest
that the throwing function is called stacktrace::throw_exception().
Add an example with basic_stacktrace that uses a custom allocator.
Add a simpler example that uses std::terminate(). The current one
is rather overwhelming. Notice that I am not asking for the current
example to be removed.
Add a note to Getting Started that the stored backtrace.dump only is
valid if the program has not be re-compiled between runs, as this is
what developers do all the time.
I have no opinion about windows.h.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk