On Sun, Dec 16, 2012 at 12:50 AM, Janus Weil <ja...@gcc.gnu.org> wrote:
> Hi,
>
>>> So, in principle I'm fine with all your BACKTRACE_* variants (except
>>> for _splurge, maybe ;)
>>>
>>> Or, why not just (plain and simple) "BACKTRACE"?
>>
>> The name is the same as backtrace() in glibc, but otherwise, sure why
>> not. _show/_print might be preferable in the sense that they convey
>> that stuff will be directly printed on the screen, rather than, say,
>> the procedure returning an array of strings with the stack trace info.
>
> Agreed. Let's go with BACKTRACE_SHOW.
>
> Attached is a new patch which uses this name. Moreover, it follow your
> previous advice to move the message "Backtrace for this error" out of
> backtrace_show into backtrace_handler. I also added "Program aborted.
> Backtrace:" in sys_abort.
>
>
>>>> - As previously show_backtrace() was always followed by program
>>>> termination, we now need to ensure that it properly cleans up after
>>>> itself in case the application continues execution. In particular,
>>>> make sure it doesn't leak file descriptors, and that the addr2line
>>>> child process terminates properly.
>>>
>>> Good point. Do you have any particular suggestions about what would be
>>> needed in this direction? (You're probably much more familiar with the
>>> libgfortran code than I am.)
>>
>> As a simple test, something like the following (untested) code might do:
>>
>> program b
>> integer :: i
>> do i = 1, 100
>> call backtrace_show
>> end do
>> read(*, *)
>> end program b
>>
>> When the programs waits on user input, check with "ps -eFH" that your
>> a.out process (or whatever you call the binary) doesn't have any child
>> processes, then "ls /proc/[PID]/fd" and check that the process has
>> only 3 fd's (std{in,out,err}).
>
> Ok, I tried this and indeed there seem to be no child processes left.
> However, I do see open fd's (one for each backtrace invocation).
> Looking at the code, it seems a "close (f[0])" was missing (which I
> added now).
Great, thanks for fixing this!
> Do you have any further comments or do you think the patch is ok for trunk
> now?
Ok for trunk. A minor addition, if you care, would be to mention in
the documentation for backtrace_show() that the error message is
printed to the unit corresponding to ERROR_UNIT in ISO_FORTRAN_ENV.
Thanks for the patch!
--
Janne Blomqvist
