[m-dev.] for review: new debugger command set (part 1 of 4)

[Zoltan Somogyi]

> > 	env = getenv(env_var);
> > 	if (env == NULL) {
> > 		return FALSE;
> > 	}
> > 
> > 	buf = checked_malloc(strlen(env) + strlen(filename) + 2);
> > 	(void) strcpy(buf, env);
> > 	(void) strcat(buf, "/");
> > 	(void) strcat(buf, filename);
> 
> The use of "/" as a directory separator is system-specific.
> On some systems, it should be "\\" or ":" instead.
> You should define a macro called say MR_DIRECTORY_SEPARATOR
> in mercury_conf.h and use that.

That would work on some non-unix systems, but not all. The proper
solution should be more generic. For example, the right way to find
the user's home directory may not be getenv("HOME"); the user may not
*have* a home directory.

For the time being, I will leave this as it is, with a note.

> > static MR_Next
> > MR_trace_debug_cmd(char *line, MR_Trace_Cmd_Info *cmd,
> > 	const MR_Stack_Layout_Label *layout, Word *saved_regs,
> > 	MR_Trace_Port port, int seqno, int depth, const char *path,
> > 	int *ancestor_level, int *max_mr_num, Code **jumpaddr)
> > {
> 
> This function is very long and has a lot of variables.
> It would be nice if you could split it into smaller functions.

Not really. Most of this function is a long chain of if-then-elses,
in which each case is fairly small, so making them into function calls
will not gain much, and require extra stuff elsewhere. The cases are
also quite similar in several respects, and it is more convenient
to have them in one place.

> Those lines could be put into a separate function called say
> MR_expand_aliases().

I did do this, however.

> It would be better for all error messages to go through a single function
> called e.g. `MR_trace_report_error' or `MR_trace_err_printf';
> this function could add the "mdb: " prefix, and it could potentially
> print the results to somewhere other than stdout.

In a later change.

Zoltan.