[m-dev.] Fix a bug caused by one of my recent changes

[Fergus Henderson]

On 22-Feb-1999, Erwan Jahier <Erwan.Jahier at irisa.fr> wrote:
> 
> One of my recent change broke the compiler in asm_fast.gc.tr.debug grade. It
> was due to the fact that I have forgotten to add print_stack_trace() as an
> argument of the occurence of MR_dump_stack_from_layout() that was inside the
> body of MR_dump_stack(); which means that I need to pass the address of
> MR_dump_print_stack_trace() down to MR_dump_stack(). But MR_dump_stack() is
> also called in the library, and in order to respect the invariant "trace ->
> library -> runtime" we can't call MR_dump_print_stack_trace() from there. So I
> move the definition of the functions the performs stack print in
> runtime/mercury_stack_trace.c.
> 
> Estimated hours taken: 2
> 
> Moves the definition of MR_dump_stack_record_print() and
> MR_dump_stack_record_print_to_socket() from trace/ to runtime/ to be able to
> call MR_dump_stack() from the library.  Pass down a flag (of type
> MR_stack_print_target) that tells which stack printing function to use.

Why not just make that type a function pointer rather than a flag?

That would be slightly simpler, and also a bit more extensible.

Also, by doing it that way, you would only need to move
MR_dump_stack_record_print() to the runtime, not
MR_dump_stack_record_print_to_socket(), which seems to me like a good
thing, since the socket stuff is really part of the external debugger,
not the runtime, and it would be better if we can keep it that way.

(MR_dump_stack_record_print was originally in the runtime
before your earlier change that turned out to break things,
so my suggested alternative fix would mean just moving it back again.)

> --- mercury_stack_trace.c	1999/02/20 06:58:06	1.27
> +++ mercury_stack_trace.c	1999/02/22 14:24:56
> @@ -49,7 +55,8 @@
>  		layout = label->i_layout;
>  		entry_layout = layout->MR_sll_entry;
>  		result = MR_dump_stack_from_layout(stderr, entry_layout,
> -			det_stack_pointer, current_frame, include_trace_data);
> +			det_stack_pointer, current_frame, include_trace_data,
> +			FILE_POINTEUR);

What's the FILE_POINTEUR?

Variable names should be in English, please.

> +/*
> +** MR_dump_stack_record_print() and 
> +*/

That comment is incomplete.

> +#ifdef MR_USE_EXTERNAL_DEBUGGER
> +
> +void    
> +MR_send_message_to_socket_format(const char *format, ...)
> +{
> +	va_list args;
> +
> +       	va_start(args, format);
> +       	vfprintf(MR_debugger_socket_out.file, format, args);
> +       	va_end(args);
> +       	fflush(MR_debugger_socket_out.file);
> +       	MR_debugger_socket_out.line_number++;
> +}

Won't that give you a compile error, since MR_debugger_socket_out
is undeclared here?

You could fix that by moving the declaration of MR_debugger_socket_out
from trace/mercury_trace_external.c to the runtime, but I think a better
fix is the one I suggested above -- pass a function pointer instead of
a flag down to the stack dumping code, and leave the external
debugger stuff in mercury_trace_external.c.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "Binaries may die
WWW: <http://www.cs.mu.oz.au/~fjh>  |   but source code lives forever"
PGP: finger fjh at 128.250.37.3        |     -- leaked Microsoft memo.