Hello, This is my review of stacktrace library: Note that I have experience with the subject as I developed a similar library for the CppCMS framework [1]
- What is your evaluation of the design?
The design is straight forward and I don't see any major issues with the API. Also I have several suggestions in handling backtrace with catch from } catch (const traced& e) { std::cerr << e.what() << '\n'; if (e.trace) { std::cerr << "Backtrace:\n" << e.trace << '\n'; } } catch (const std::exception& e) { std::cerr << e.what() << '\n'; } In my library I use much simpler approach in terms of API [2] catch(std::exception const &e) { std::cerr << e.what() << std::endl; std::cerr << booster::trace(e); } Where booster::trace is a manipulator that prints strack trace if "e" contains backtrace information - much shorted code without additional catch. I find it very easy to use. Just a suggestion. I also like with_trace templated class idea.
- What is your evaluation of the implementation? Most of my personal concerns with this library are with the implementation and I would hugely appreciate feedback from others with substantial experience of using stacktracing "in anger" in non-trivial use case scenarios.
I mostly reviewed the backend. Linux: ========= 1st of all: I recommend mentioning in the documentation the option -rdynamic/-export-dynamic as otherwise dladdr would likely to fail - it is critical as many may miss one. Regarding use of add2line utility: 1. Failing to calling an external program in case of dladdr fails is bad idea. This behaviour is unexpected by the user. I understand the advantage of this as you can use debug information but should be done only per user specific request (i.e. compilation option) and not by default. 2. If you call add2line - use it for ALL addresses in backtrace at once and not for each single one! My recommendation is to provide alternative backed that uses other way to extract information not involving forking. Maybe try to analyze debug data or so - or require preparing initial map using nm tool and than loading it from file or something like that. Additionally calling the backed linux is bad - as same approach with slight modifications works on *BSD, Mac OS X and Solaris (RIP). Windows: ======== I have a question why you used COM instead of SymFromAddr functions? I understand thread safety issues but simple mutex can solve them. My concerns are regarding MinGW compilers. Have you tested it on MinGW compiler? It wouldn't work as it used MSVC debug information in general so maybe it is good idea to provide an alternative backend that extracts only address and DLL name so you can analyse the stacktrace later (using addr2line utility for example) Take a look on my code [1] regarding non-msvc windows compilers - it provides at least some useful information.
- What is your evaluation of the documentation?
The documentation is ok in general and straight forward as the library itself. What I do lack is the documentation of underlying backends - how do they work. 1. use of -rdynamic/-export-dynamic should be noted in "bold face" regarding the use of the library. 2. please do not call it "POSIX backend" - as there none of the stuff you use is actually POSIX... Also you'll probably need different backends for MINGW and mention it.
- What is your evaluation of the potential usefulness of the library?
Very Very useful.
- Did you try to use the library? With what compiler? Did you
have any problems? No I didn't - I mostly read the documentation and the backend code.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
3-4 hours
- Are you knowledgeable about the problem domain?
Yes I am. I have written similar library for CppCMS project [1] - FINAL VOTE: Yes, Conditionally Conditions: Use of addr2line must not be enabled by default. [1] https://sourceforge.net/p/cppcms/code/HEAD/tree/framework/trunk/booster/boos... https://sourceforge.net/p/cppcms/code/HEAD/tree/framework/trunk/booster/lib/... [2] https://sourceforge.net/p/cppcms/code/HEAD/tree/framework/trunk/booster/boos... Best Regards, Artyom Beilis