Re: [PATCH 14/18] WTX-TCL support module

Tom Tromey
2011-02-25T15:33:26Z

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> For VxWorks 5.x and 653, we need to use the TCL extension in order to
Joel> access some of the information we are looking for (list of VxWorks
Joel> tasks running on the target, for instance). This is only because
Joel> the WTX protocol does not provide access to this info.

Joel> +/* Includes from the VxWorks install. */
Joel> +#define HOST
Joel> +#include "tcl.h"
Joel> +#if WTX_PROT_VERSION != 4
Joel> +#include "wtxtcl.h"
Joel> +#endif

I guess this explains why the new code isn't part of
--enable-targets=all. I am somewhat concerned that this will lead to
bit-rot. I always build using this option in an attempt to avoid
breaking things; but this code will not be included in that.

I don't really see a way around it, though, unless you want to
virtualize all the wtx calls.

I guess I want us to be clear that build breakage for this is expected.

Joel> +static int
Joel> +wtxtcl_eval_verbose (char *str)
Joel> +{
Joel> + char *output;
Joel> + const int success = wtxtcl_eval (str, &output);
Joel> +
Joel> + if (success)
Joel> + printf_filtered ("%s\n", output);
Joel> + else
Joel> + printf_filtered (_("TCL error: %s\n"), output);
Joel> +
Joel> + return success;
Joel> +}
Joel> +
Joel> +/* Implement the "tcl" command. */
Joel> +
Joel> +static void
Joel> +tcl_command (char *args, int from_tty)
Joel> +{
Joel> + if (args == NULL)
Joel> + return;
Joel> +
Joel> + wtxtcl_eval_verbose (args);

Why not call error instead of just printing a message when a Tcl command
fails?

Joel> + tcl_cmd = xstrprintf ("taskInfoGet 0x%x", task_id);
Joel> + success = wtxtcl_eval (tcl_cmd, &task_info);

Nothing ever frees tcl_cmd.

Joel> + /* Skip the first 8 tokens and go directly to the 9th, which contains
Joel> + the PD ID. */
Joel> + for (j = 0; j < 8; j++)
Joel> + task_info = skip_space_delimited_token (task_info);

This is the wrong way to parse a Tcl list. It may work ok for your
purposes, if you know that the result can never include anything "weird".
But it is easy and safer to just use Tcl_SplitList.

Joel> + /* Parse the result. */
Joel> +
Joel> + current_thread = skip_whitespace (tcl_output);
Joel> + while (current_thread && *current_thread != '\0')
Joel> + {
Joel> + struct wtxapi_thread_info *new_thread
Joel> + = xmalloc (sizeof (struct wtxapi_thread_info));
Joel> +
Joel> + /* Get the thread id. */
Joel> + new_thread->id = strtoul (current_thread, NULL, 0);
Joel> + current_thread = skip_space_delimited_token (current_thread);
Joel> +
Joel> + /* Get the thread name. */
Joel> + current_thread = skip_whitespace (current_thread);
Joel> + if (*current_thread == '{')
Joel> + {
Joel> + /* The thread name delimited by curly braces. Find the
Joel> + closing curly brace. */
Joel> + char *start = current_thread + 1; /* skip the '{'... */
Joel> + char *end = skip_until_character (current_thread, '}');
Joel> + char tmp = *end;

Likewise.

Joel> + add_com ("tcl", class_obscure, tcl_command,
Joel> + _("Evaluate the arguments with the TCL interpreter"));

I am not super fond of a top-level command named "tcl".

Tom