On Tue, Apr 4, 2017 at 6:50 AM, <bas...@starynkevitch.net> wrote: > On 2017-04-04 15:38, Ian Lance Taylor wrote: > >> How about we just add backtrace_destroy_state? > > I don't know how to code that. In my > https://github.com/bstarynk/melt-monitor I observed that calling free on > such > a struct backtrace_state pointer is breaking things.
Well, yes, it would have to call backtrace_free. But more than that it would have to munmap the free list. So you're right that it's not trivial. > I also find the code of backtrace_create_state a bit complex (maybe for > historical reasons). Why does it call backtrace_alloc instead of just > calling malloc? Because backtrace_create_state, like all the backtrace functions, can be called from a signal handler. The backtrace code can never call malloc. (It does call malloc on systems that do not support anonymous mmap, because there is no other choice, which means that the backtrace library does not really work entirely correctly on such systems. Fortunately such systems are rare these days. See BACKTRACE_USES_MALLOC.) > And why would it call the error_callback on failure? (I would just return > NULL in that case, leaving the caller of backtrace_create_state to handle > that out-of-memory error itself). It will call the error_callback on mmap failure, or an attempt to pass threaded as true when it is not supported. > Actually, I tend to believe that backtrace_create_state should have its > signature changed to just: > > struct backtrace_state *backtrace_create_state (const char *filename, int > threaded); > > Or maybe the above should be called backtrace_create_simple_state? We could just document and implement passing error_callback as NULL. Ian
