On 2017-04-04 15:38, Ian Lance Taylor wrote:
On Tue, Apr 4, 2017 at 5:05 AM, <bas...@starynkevitch.net> wrote:
I just discovered that backtrace_create_state should be called once,
that it
is returning some heap-allocated data (which cannot be free-d, because
there
is no
backtrace_destroy_state routine).
I suggest the attached patch (against GCC trunk r246678) which just
improves
the comment describing that function.
You are adding that backtrace_create_state should be called "(probably
at startup, e.g. early in main)"? But that is not accurate. It's
perfectly reasonable to do what GCC itself does, which is call
backtrace_create_state only when it encounters an internal compiler
error (in diagnostic_action_after_output in gcc/diagnostic.c).
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.
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?
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).
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?
BTW, I guess that changing the API is not possible in current stage
(that it why I suggested just a comment change).
Cheers
--
Basile Starynkevitch (France) http://starynkevitch.net/Basile/
